mixxx
mixxx copied to clipboard
Renamed symbols (Parameter -> NormalizedValue) and added aliases for controller script backward compatibility
In the COs and the controller script API, the meaningless term "Parameter" was systematically used instead of the obviously correct term "Normalized Value". This is particularly problematic as it is a symbol that is exposed to the user through our controller scripting API. A user has no chance to understand, that "Parameter" indicates a value with normalized range - unless he looks up in the documentation for it.
I renamed the affected methods therefore, and added aliases to the controller scripting API for backward compatibility.
Additionally I expanded the term Midi in some CO methods to Midi7Bit, as nowadays Midi allows larger ranges than 0...127. But this range is meant here.
Last change was extending the controllerscriptenginelegacy_test.cpp to cover the previously uncovered methods which were renamed, and a check for the aliases itself.
Thank you for picking this up.
Unfortunately It is not always the normalized value. In case of knobs and sliders, it reflects the potentiometer position normalized to the range of 0 ... 1. For all other COs it is equal to value.
I am a bit torn if the term "NormilizedValue" just shifts the confusion. Maybe we can find a name the emphasize that this is only relevant for sliders and knobs. Something like PotentiometerPos, PotPosition, controlPosition, or just position.?
What do you think?
For all other COs it is equal to value.
In these cases I would suggest to print a warning that set/getNormalizedValues is not supported for non-Pot-Meter-COs, and that set/getValue should be used for this CO instead.
For all other COs it is equal to value.
In these cases I would suggest to print a warning that set/getNormalizedValues is not supported for non-Pot-Meter-COs, and that set/getValue should be used for this CO instead.
Good idea. In these case the "parameter" value should not be used.
In these cases I would suggest to print a warning that set/getNormalizedValues is not supported for non-Pot-Meter-COs, and that set/getValue should be used for this CO instead.
Good idea. In these case the "parameter" value should not be used.
Sure, backward compatibilty is prio no. 1 for everything controller mapping related. I set the PR to draft, until I've addressed this.
Did you consider to adjust the name?
Did you consider to adjust the name?
To be honest, neither Potentiometer nor Position is related to what the function does, and to have this selfdescriptive name is the sole reason I work on this.
Did you consider to adjust the name?
To be honest, neither Potentiometer nor Position is related to what the function does, and to have this selfdescriptive name is the sole reason I work on this.
Normalization to 0..1 alone is IMHO not descriptive enough. For my understanding it is always scaled to the linear position of the slider / knob scale or otherwise defaults to value. This is at least true for rate, pregain and volume.
Normalization involved always min and max value and a scale. In this case min and max are the end positions of the potentiometer and the scale linear 0...1.
Please give examples where neither Potentiometer nor Position is related.
An example is the normalized value returned by engine.getParameter is useful for VU-Meter, just multiply it by the number of LEDs in your controler and you get what you need to light them up in the right brightness.
The typical use case for the normalization done by engine.getParameterForValue is range checking in an assert like code.
The problem I'm addressing with this PR is that, as in any good API, the other function names of the controller scripting API describe what they do/implement, but not the "Parameter" ones. Since terms like "location" or "potentiometer" don't do this either, this would contradict the intent of this PR.
The 'vu_meter' CO is a plain ContolObect it has no potentiometer position assisted and using get/setPatameter may issue the discussed warning.
The typical use case for the normalization done by engine.getParameterForValue is range checking in an assert like code.
This does not work for plain ControlObejects and may issue a warning if dome anyway.
I agree tin general, hat parameter was a name without any meaning so let's find a name that solves the issue. NormalizedValue just shifts the problem.
The relationship is defined here: https://github.com/mixxxdj/mixxx/blob/2145f5f7dec746ab314fa136ca2707e15a3a620b/src/control/controlbehavior.cpp We may go through it one by one and check a possible name.
Since it automatically selects the normalization mode depending on the CO type, what about "AutoNormalizedValue" ?
There term "normalization" does not fit well IMHO.
We need a name for this
- Null (Plain VonzrolObject): value
- ControlNumericBehavior: value
- ControlPotmeterBehavior: pot position
- ControlLogPotmeterBehavior: pot position
- ControlLogInvPotmeterBehevior: pot position
- ControlLinInvPotmeterBehavior: pot position
- ControlAudioTaperPotBehavior: pot position
- ControlTTRotaryBehavior: ?
- ControlLinSteppedIntPotBehavior: pot position
- ControlPushButtonBehavior: value
Can you describe what the function does in other terms? I never heard another term than normalization for this operation.
The parameter/value pair has been introduced to allow simple mappings of the controller hardware to the values in Mixxx.
The abstract model of such a control is something like this:
Min ----- Center ----- Max
where min in represented as 0.0 center as 0.5 and max as 1.0.
The control behaviour is used to "map" this model to the value of the business logic, like the "rate" multiplier in the range of -1, 0 and +1 or in case of volume the logarithmic 0, 2.4 and 1.
The advantage is that this logic is part of Mixxx and there is no need to implement it in mappings. This makes sure that controller and GUI sliders behave the same.
The mapping author can override it by changing the "value" directly.
This PR is marked as stale because it has been open 90 days with no activity.