Skip to content
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

Two fixes to OSCSelectObserver: apply feedback config and fix send/plugin paging #900

Conversation

matthijskooijman
Copy link
Contributor

These are two small fixes to the OSC selected strip code that I ran into while testing for another (to be submitted) PR. Since these are small and unrelated fixes, it seemed good to submit these separately. See commit messages for details.

Before this commit, OSCSelectObserver would read the feedback value when
it was created, but then never update it anymore. In practice, the
OSCSelectObserver is created on startup, and when the surface connects
and configures feeback, this value is not applied.

For example, when sending `/set_surface` with a feedback value of
4 (Send SSID as path extension), `/strip/*` would get their ssid put
into the path, but `/select/plugin/*` messages would not have their
parameter id in the path. When setting the corresponding checkbox in the
default feedback preferences, it is applied as expected.

This commit passes the new feedback value to the OSCSelectObserver
instance whenever it changes, which ensures the value is applied as
expected.
When handling the `/set_surface` command, the code would set
plug_page_size to the new value first, and call `sel_plug_pagesize()`
later. The latter then sees the page size is already the same, so it
leaves it unchanged and also does not send the page size to the
OSCSelectObserver object. In practice, this means that only the default
plugin page size from the preferences or set with
`/set_surface/plugin_page_size` take effect and values set with
`/set_surface` are ignored.

Exactly the same thing happens for the send page size.

This code has been like this since it was first introduced in comit
9c0f6ea (OSC: Allow set_surface to set send and plugin page sizes.,
2017-06-13)

This commit fixes this by omitting the first assignment.
@x42
Copy link
Member

x42 commented Sep 2, 2024

Sorry for the delay, rebased and merged as 8.6-502 a9a5787

@x42 x42 closed this Sep 2, 2024
@matthijskooijman
Copy link
Contributor Author

No worries, thanks for merging!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants