-
Notifications
You must be signed in to change notification settings - Fork 2
Don't show settings that aren't published by device [CPP-587] #372
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
Conversation
Changes the settings tab to only show the settings that the device has published. This avoids settings being shown that exist within the libsettings `settings.yaml` that are old or are for a different mode of operation.
I think this code was just added by @notoriaga -- so we should make sure we aren't undoing a recent fix |
As far as I can tell, this preloading of every setting has been around since the first version of the settings tab: e7c5d73#diff-3781a73d3071fa7b8243bfc06e2ae8ff889747ecb6438ea0b0f91908abbce6b7R356 (though the data structure was a bit different). But maybe there's a special reason for doing so that I'm missing? |
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.
Hmmm I don't recall why I did it that way, I think it's probably safe to remove
Added some functionality to preserve ordering of groups and settings within a group as discussed here: https://swift-nav.atlassian.net/browse/CPP-563 |
.collect(); | ||
|
||
// Sort settings within a group by the order in which they're defined within the libsettings settings.yaml file | ||
group.sort_by(|a, b| { |
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.
I am a little puzzled as to how this is working with SETTING_ORDERING being a HashMap instead of IndexMap but testing it out passing the settings-yaml cli argument maintained correct group order.
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.
Ah, SETTING_ORDERING
is only holding the position in the YAML file of each setting, so it doesn't matter how its ordered.
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.
Ah I see now that makes sense.
Might have a two for one here https://swift-nav.atlassian.net/browse/CPP-607 I'm guessing the empty values are from settings not returned by the device? |
Changes the settings tab to only show the settings that the device has
published. This avoids settings being shown that exist within the
libsettings
settings.yaml
that are old or are for a different mode ofoperation.
Tested by making sure I didn't see any INS entries when connecting to a non INS piksi.