-
Notifications
You must be signed in to change notification settings - Fork 80
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Improved device parameter support #114
Conversation
- Added individual parameter listeners
- Added new endpoint for getting a human readable value string for a parameter value (ex: "2500Hz", "Lowpass")
- Added device selector endpoint (handy for "focusing" onto a device from a controller)
- Updated README
- Note: I didn't add an automated test because the guidelines call out using the default Ableton project, but the default doesn't include any devices to test with.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice additions, thanks @steeltrack! Added a few comments inline.
One reservation is that the parameter value listener results in two responses for every query (one for value
and one for value_string
), which will double the network traffic for what might already be quite high-volume messages. Have you seen any issues with performance with this?
The alternative would be to have separate start_listen
endpoints for value
and value_string
, which would be more economical, and consistent with the rest of the AbletonOSC API (in which one listener generates one response). I'd prefer this on balance, as users will only want either the value
or the value_string
, and it's better to minimise traffic where possible as I want to make sure this library can support large sets with a lot of activity.
@ideoforms Thanks for all of the feedback. I've addressed the inline comments in my latest commit. To your larger point, str_for_value is a conversion method, not a stored value. Therefore, human readable parameter value strings have no listeners of their own. This is why I grouped both values into one listener even though it increases the amount of network traffic. One alternative would be exposing the conversion method as its own OSC endpoint, but then you have the network traffic of sending the float value and getting the string response. For what it's worth, I didn't notice performance issues. Also, I somewhat disagree with this point: "users will only want either the value or the value_string." In order to replicate the Live control below, you must have the parameter float values to transact with the API, which you can then convert to a human-readable string. However, that human-readable string is not actionable (ie: it can't be sent back to the API to change a parameter). ![]() Let me know your thoughts. Thanks again for the thorough review! |
All of that sounds reasonable, and changes look good. Thanks, merging now! |