[CPP-230] Add serial history #168
Conversation
Stores serial history as part of the connection history. This allows the front end to: - Default to having the most recently used serial port selected, with the most recent settings. - Change the settings to be the most recently used settings for the selected port.
- Increases the size of the serial combo box and connection type combo box, to avoid text getting cut off - Removes the changing of width on the combo box icon - Sets the refresh button to the font awesome icon
| Component.onCompleted: { | ||
| serialDeviceBaudRate.indicator.width = Constants.navBar.serialDeviceBaudRateDropdownWidth / 3; | ||
| } |
There was a problem hiding this comment.
I'm not sure the reason for this change of width for the icons, but it was making the combo box icons skewed to me, and removing them doesn't seem to have degraded anything as far as I can tell. Maybe you know @john-michaelburke ? Let me know if they're needed and I can revert the removals.
There was a problem hiding this comment.
These may have been artifacts from when we were using a much smaller window height/width and have since not been removed. IIRC there was some wonky eliding happening and this fixed it. Nonetheless seems fine to remove.
console_backend/src/shared_state.rs
Outdated
| /// - `flow`: The chosen flow control | ||
| pub fn record_serial(&mut self, device: String, baud: u32, flow: FlowControl) { | ||
| self.serials.shift_remove(&device); | ||
| self.serials.insert(device.clone()); |
There was a problem hiding this comment.
This is kind of ugly, but the simplest way I could think to implement this for the moment. Basically the self.serials is storing the list of serial devices in order of last use and self.serial_configs is storing the associated configs for each one.
I might have a go at only using one data structure to do this tomorrow, or at least change it so that only the map needs to be serialised rather than both the map and the IndexSet, but I'm open to any ideas as well.
There was a problem hiding this comment.
I wonder if you could reduce this logic slightly with the use of an indexmap: https://docs.rs/indexmap/1.7.0/indexmap/
I guess I could see the ability to remove "self.serials" in place of a single indexmap in which you still would need to do the split logic but would not need to remove from the other container. Although I'm not sure if inserting to overwrite would count as updating the insertion order. Might have to play with it.
There was a problem hiding this comment.
Ah nice, thanks - will have a try with the indexmap
There was a problem hiding this comment.
Have implemented with a IndexMap now, and it's much nicer! Thanks @john-michaelburke
| NAV_BAR[Keys.PREVIOUS_FILES][:] = m.navBarStatus.previousFiles | ||
| NAV_BAR[Keys.LOG_LEVEL] = m.navBarStatus.logLevel | ||
| NAV_BAR[Keys.LAST_USED_SERIAL_DEVICE] = ( | ||
| m.navBarStatus.lastSerialDevice.port if m.navBarStatus.lastSerialDevice.which() == "port" else None |
There was a problem hiding this comment.
What is the logic behind this line? Ah I see this is an optional capnproto message field.
console_backend/src/shared_state.rs
Outdated
|
|
||
| #[derive(Serialize, Deserialize)] | ||
| #[serde(remote = "FlowControl")] | ||
| enum FlowControlDef { |
There was a problem hiding this comment.
Maybe calling this FlowControlRemote would be more suitable unless you have a reason for calling it Def I don't understand.
There was a problem hiding this comment.
Can do. I was using the conventions I saw in the serde documentation: https://serde.rs/remote-derive.html but I'm not particularly attached to using Def, will change it.
There was a problem hiding this comment.
Interesting, I was not aware. Still seems ambiguous to call it a definition. 🤷
Much neater than the previous approach of using and IndexSet and a HashMap!
2 participants
Stores serial history as part of the connection history. This allows the front end to:
Also made some small tweaks to the connection bar (some (but I don't think all) of which I've since realised will be superseded by JM's open change: #167).
Demo (
/dev/ttyUSB1was the last serial device used @ 921600 with hardware flow control, but/dev/ttyUSB0was also used previously @ 460800 with software flow control):