[CPP-50][CPP-217]Advanced Magnetometer Tab. #73
Conversation
john-michaelburke
commented
Jun 16, 2021
john-michaelburke
commented
- Adds a magnetometer tab under advanced.
- Converts the consts in constants.rs to pub(crate) consts.
| let shared_state = SharedState::new(); | ||
| let client_send = TestSender { inner: Vec::new() }; | ||
| let mut mag_tab = AdvancedMagnetometerTab::new(shared_state, client_send); | ||
| sleep(Duration::from_secs_f64(GUI_UPDATE_PERIOD)); |
There was a problem hiding this comment.
if GUI_UPDATE_PERIOD is moved into a variable of either the tab or maybe client_send, it could be adjusted during test execution to avoid these sleeps.
Or, I guess/wonder if the value of the constant GUI_UPDATE_PERIOD can be reduced to a lower value during test invocation only. It seems const functions would allow this, combined with if cfg!(test) {...} else {...}
There was a problem hiding this comment.
I think that is a good idea. I ended up removing it though because we don't need the rate limiting AFAICT. This is a feature in other tabs though I'll keep in mind when we need it. Could be good for the tracking tab as we decided to add it back there recently.
There was a problem hiding this comment.
Why shouldn't this tab/all tabs have rate limiting? It seems like a useful approach in general to avoid the possibility of the GUI becoming lagged, esp. as we already need it for some tabs. Anyways, this doesnt need to hold up this PR.
There was a problem hiding this comment.
The old console sort mixed the rate limiting of the backend and the frontend. Using the GUI_UPDATE_PERIOD here would be in some sense rate limiting how frequently the backend sends data to the frontend. Our QML frontend does impose it's own rate limiting using the timers. I don't argue that there could be value in rate limiting the backend and its comms with the frontend but we have not really had issue with it so far so I think we can leave it on the backburner for now. It would be good to reassess things like this once we approach initial launch to see if we can optimize things so I'll create a ticket for it. https://swift-nav.atlassian.net/browse/CPP-228
The use of the GUI_UPDATE_PERIOD in the tracking signals was not imposed to reduce lagging but rather we were collecting so many points the xaxis moving too quickly so instead of averaging things over time we reverted back to the GUI filter from the old console. (I know you saw this already but I'm going to link the above ticket to this comment and reference to the other PR here): The discussion is somewhat scattered throughout this PR and referred to as implementing decimination: #67
There was a problem hiding this comment.
In the case of the tracking tab GUI_UPDATE_PERIOD is probably a bad name, as you mention, we use timers in most of the GUI to drive the refresh rate of the plots: the back-end data gets pushed to the front-end data model whenever there's new data available, then the GUI pulls the data according to the timer period (which again, is driven by the refresh rate). The GUI_UPDATE_PERIOD constant should probably be called TRACKING_UPDATE_PERIOD.
There was a problem hiding this comment.
I swapped out the naming from GUI to TRACKING.
|
In the legend, it looks like there should be some extra whitespace between the line and description. |
3 participants
