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

topquark12 · 2021-01-29T12:13:51+08:00

topquark12 commented

2021-01-29 12:13:51 +08:00

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

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

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

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

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

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

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

355f10c713 main: update comment

sb10q merged commit a5d8661b10 into master

2021-01-29 16:18:07 +08:00

sb10q referenced this issue from a commit

2021-01-29 16:18:08 +08:00

main: fix handling of incomplete data received (#55)

topquark12 deleted branch fix_double_brac

2021-01-29 16:22:34 +08:00

Sign in to join this conversation.

No reviewers

No Label

No Milestone

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