M-Labs/thermostat
9
0
Fork 5
Code Issues 9 Pull Requests 5 Releases Wiki Activity

main: fix double brackets sent when socket rx ring buffer wraps around #55

Merged
sb10q merged 2 commits from fix_double_brac into master 2021-01-29 16:18:07 +08:00
Conversation 0 Commits 2 Files Changed 2 +4 -3
1 changed files with 4 additions and 3 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files
Showing only changes of commit 355f10c713 - Show all commits

7
src/main.rs
View File

@ -213,9 +213,10 @@ fn main() -> ! {
socket.close() socket.close()
} else if socket.can_send() && socket.can_recv() { } else if socket.can_send() && socket.can_recv() {
match socket.recv(|buf| session.feed(buf)) { match socket.recv(|buf| session.feed(buf)) {
// SessionInput::Nothing happens when socket RX ring buffer wraps around without // SessionInput::Nothing happens when the line reader parses a string of characters that is not
sb10q commented 2021-01-29 15:45:01 +08:00
Outdated
Review
Copy Link

Your code seems correct but this comment sounds inaccurate: isn't SessionInput::Nothing also returned in other situations, e.g. if you send a partial command without the newline (irrespective of whether the ring buffer wrapped around or not)?

Your code seems correct but this comment sounds inaccurate: isn't `SessionInput::Nothing` also returned in other situations, e.g. if you send a partial command without the newline (irrespective of whether the ring buffer wrapped around or not)?
topquark12 commented 2021-01-29 15:56:22 +08:00
Outdated
Review
Copy Link

Hm, I have only considered the use case of using Netcat or the pytec client library to interface the device, where every command will always be followed by a newline.

What you just said is certainly true if someone decides to roll their own application to interface with the thermostat. I'll update the comment.

Hm, I have only considered the use case of using Netcat or the pytec client library to interface the device, where every command will always be followed by a newline. What you just said is certainly true if someone decides to roll their own application to interface with the thermostat. I'll update the comment.
sb10q commented 2021-01-29 15:58:39 +08:00
Outdated
Review
Copy Link

where every command will always be followed by a newline.

No they won't. TCP doesn't guarantee that data will not be fragmented. For instance, if one side of the transmission sends "1234" then the other may receive "12" first and then "34" later. TCP data should be treated as a pure stream.

> where every command will always be followed by a newline. No they won't. TCP doesn't guarantee that data will not be fragmented. For instance, if one side of the transmission sends "1234" then the other may receive "12" first and then "34" later. TCP data should be treated as a pure stream.
topquark12 commented 2021-01-29 16:06:08 +08:00
Outdated
Review
Copy Link

Right, I am surprised this bug didn't occur more often then, given all the things that could have gone wrong.

Right, I am surprised this bug didn't occur more often then, given all the things that could have gone wrong.
// the line reader reading a newline character, should do nothing and let // followed by a newline character. Could be due to partial commands not terminated with newline,
// the line reader read from the start of ring buffer in the next loop cycle. // socket RX ring buffer wraps around, or when the command is sent as seperate TCP packets etc.
// Do nothing and feed more data to the line reader in the next loop cycle.
Ok(SessionInput::Nothing) => {} Ok(SessionInput::Nothing) => {}
Ok(SessionInput::Command(command)) => match command { Ok(SessionInput::Command(command)) => match command {
Command::Quit => Command::Quit =>