More reliable conn logic/serverstate->nonarcmutex[CPP-251] #87
Conversation
john-michaelburke
changed the title
john-michaelburke
force-pushed
the
john-michaelburke/unlimited_connection
branch
from
8187d24 to
e5541f1
Compare
john-michaelburke
changed the title
| let (rdr, _) = conn.into_io(); | ||
| let mut main = MainTab::new(shared_state.clone(), client_send); | ||
| let realtime_delay = conn.realtime_delay(); | ||
| let (rdr, _) = conn.try_connect(Some(shared_state.clone()))?; |
There was a problem hiding this comment.
I have moved the creation of the reader/writer into process_messages here.
| debug!("{}", e); | ||
| match e { | ||
| sbp::Error::IoError(err) => { | ||
| if (*err).kind() == ErrorKind::TimedOut { |
There was a problem hiding this comment.
This approach seems to work but may be a little finicky. Will likely need to switch to a match handling error types which correspond to logic we want to result in reconnection or closing the connection. As it is now it seems to handle internet dropping for TCP connections but exits if a serialport has a timeout.
john-michaelburke
changed the title
console_backend/src/types.rs
Outdated
| pub rdr: Box<dyn io::Read + Send>, | ||
| pub wtr: Box<dyn io::Write + Send>, | ||
| #[derive(Clone)] | ||
| pub struct TcpConnection { |
There was a problem hiding this comment.
this might be an opportunity to move connection related types to a new file, like connection.rs , possibly including ServerState?
There was a problem hiding this comment.
Done. I also renamed serverstate to ConnectionState as it seemed more fitting.
console_backend/src/types.rs
Outdated
| trait ConnectionType { | ||
| fn close_when_done(&self) -> bool { | ||
| false | ||
| } | ||
| fn name(&self) -> String; | ||
| fn realtime_delay(&self) -> RealtimeDelay { | ||
| RealtimeDelay::Off | ||
| } | ||
| fn try_connect( | ||
| self, | ||
| shared_state: Option<SharedState>, | ||
| ) -> Result<(Box<dyn io::Read + Send>, Box<dyn io::Write + Send>)>; | ||
| } | ||
|
|
||
| #[derive(Clone)] | ||
| pub enum Connection { | ||
| Tcp(TcpConnection), | ||
| File(FileConnection), | ||
| Serial(SerialConnection), | ||
| } |
There was a problem hiding this comment.
I don't think you need both the trait and the enum here, they kind of accomplish the same thing. I think you can just move the methods implemented in ConnectionType into regular impl blocks for TcpConnection, FileConnection and SerialConnection. Then you do the same dispatching inside of the Connection enum methods.
The other alternative would be to make things generic over ConnectionType and then drop the enum, but that seems a bit more complicated and imo not worth it.
There was a problem hiding this comment.
Removed the trait. I'm not entirely sure what I was thinking here.
There was a problem hiding this comment.
I mean I think either way makes sense, I kind of like the enum here because we know all the variants beforehand, and then we don't have to sprinkle generic parameters everywhere
john-michaelburke
force-pushed
the
john-michaelburke/unlimited_connection
branch
from
76efd2d to
23cc00d
Compare
3 participants
Implemented
Unimplemented
Need to figure out how to kill hanging connection. See:https://snav.slack.com/archives/C020VUZ9CMP/p1625860893003600