fileio: Small improvements to time outs [DEVINFRA-625] #416
Conversation
- Update `read` to timeout if no messages are received - Fix a small bug with `write` which meant it never actually timed out - Add hints in timeout messages to tell users what messages they may need to enable in order to not have timeouts on the fileio commands. This is especially useful for MSG_FILEIO_READ_DIR_RESP which isn't enabled to be output by default.
| if let Some(req) = guard.get_mut(&seq) { | ||
| req.retries += 1; | ||
| trace!("retry {} times {}", req.message.sequence, req.retries); | ||
| if req.retries >= MAX_RETRIES { |
There was a problem hiding this comment.
Not a huge deal but this takes ~30 seconds to time out if 171 isn't enabled. Would it be unwise to lower MAX_RETRIES?
There was a problem hiding this comment.
I think the timeout duration and max retries where taken from the old console. Doesn't mean we shouldn't change it though, it does seem like a while
There was a problem hiding this comment.
I think we'd need to retest on a lossy/ slow serial connection if we want to adjust these parameters. We could mitigate the bad user experience by displaying a message after a shorter time (like ten seconds) -- something like "still waiting for READ_DIR message..."
There was a problem hiding this comment.
Will leave it for now then, I think it's probably a rare case that the message isn't enabled as it is enabled by default.
console_backend/src/fileio.rs
Outdated
| }, | ||
| recv(channel::tick(FILE_IO_TIMEOUT)) -> _ => { | ||
| self.link.unregister(key); | ||
| bail!("Timed out waiting for file read response. Ensure SBP FILEIO message 163 (MSG_FILEIO_READ_RESP) is enabled."); |
There was a problem hiding this comment.
small nit you could use MsgFileioReadResp::MESSAGE_NAME/MsgFileioReadResp::MESSAGE_TYPE etc. for all these warnings
There was a problem hiding this comment.
ah good call, have updated
3 participants
Based on some feedback from Stefan:
readto timeout if no messages are receivedwritewhich meant it never actually timed outneed to enable in order to not have timeouts on the fileio commands.
This is especially useful for MSG_FILEIO_READ_DIR_RESP which isn't
enabled to be output by default.