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

Fix settings notification again #539

Merged
merged 4 commits into from
Sep 13, 2018

Conversation

julianoes
Copy link
Collaborator

This resolves the issue where the current settings were not notified
after color mode was updated. This happened because color mode does
not lead to any other setting changes except the color mode itself.
In this case get_unknown_params() would not find any unknown params
but still return true. The right solution is to get rid of the confusing
return argument and just check the size of the params vector directly.

This is the correct fix instead of #524.

This resolves the issue where the current settings were not notified
after color mode was updated. This happened because color mode does
not lead to any other setting changes except the color mode itself.
In this case `get_unknown_params()` would not find any unknown params
but still return true. The right solution is to get rid of the confusing
return argument and just check the size of the params vector directly.
@julianoes
Copy link
Collaborator Author

@JonasVautherin I'm gonna release the backend 0.2.5 and test it.

@julianoes
Copy link
Collaborator Author

Tested, works.

Copy link
Collaborator

@JonasVautherin JonasVautherin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome!

@JonasVautherin JonasVautherin merged commit 1fb8b8c into develop Sep 13, 2018
@JonasVautherin JonasVautherin deleted the fix-settings-notification-again branch September 13, 2018 19:50
rt-2pm2 pushed a commit to rt-2pm2/DronecodeSDK that referenced this pull request Nov 27, 2018
…on-again

Fix settings notification again
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