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 +7 -8
topquark12 commented 2021-01-29 12:13:51 +08:00
Contributor
Copy Link

Fix double brackets sent when socket rx ring buffer wraps around. Fixes #42

The line of code causing the issue was actually added a month ago in b7e6cdbec2 , I wonder if there's a reason behind that change?

Fix double brackets sent when socket rx ring buffer wraps around. Fixes #42 The line of code causing the issue was actually added a month ago in b7e6cdbec2 , I wonder if there's a reason behind that change?
topquark12 added 1 commit 2021-01-29 12:13:52 +08:00
sb10q reviewed 2021-01-29 15:45:01 +08:00
src/main.rs Outdated
@ -216,3 +216,1 @@
Ok(SessionInput::Nothing) => {
send_line(&mut socket, b"{}");
}
// SessionInput::Nothing happens when socket RX ring buffer wraps around without
sb10q commented 2021-01-29 15:45:01 +08:00
Owner
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
Author
Contributor
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
Owner
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
Author
Contributor
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.
topquark12 added 1 commit 2021-01-29 16:17:07 +08:00
sb10q merged commit a5d8661b10 into master 2021-01-29 16:18:07 +08:00
topquark12 deleted branch fix_double_brac 2021-01-29 16:22:34 +08:00
Sign in to join this conversation.
Reviewers
No reviewers
Labels
Clear labels
No items
No Label
Milestone
Clear milestone
No items
No Milestone
Assignees
Clear assignees
No Assignees
2 Participants
Notifications
Due Date
The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: M-Labs/thermostat#55
No description provided.
Delete Branch "fix_double_brac"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?