Fix SDO writes of empty strings. #551
Conversation
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
❗ Your organization needs to install the Codecov GitHub app to enable full functionality. Additional details and impacted files@@ Coverage Diff @@
## master #551 +/- ##
==========================================
+ Coverage 71.36% 71.42% +0.06%
==========================================
Files 26 26
Lines 3129 3129
Branches 480 480
==========================================
+ Hits 2233 2235 +2
+ Misses 765 764 -1
+ Partials 131 130 -1
|
|
|
||
| def test_expedited_download_zero_length(self): | ||
| self.data = [ | ||
| (TX, b'\x33\x00\x20\x00\x00\x00\x00\x00'), |
There was a problem hiding this comment.
For an expedited download, the toggle bit should be zero, thus this byte sequence needs to start with 0x23, right?
| @@ -215,7 +215,9 @@ def open(self, index, subindex=0, mode="rb", encoding="ascii", | |||
| raw_stream = BlockDownloadStream(self, index, subindex, size, request_crc_support=request_crc_support) | |||
| else: | |||
| raw_stream = WritableStream(self, index, subindex, size, force_segment) | |||
| if buffering: | |||
| if buffering and ((size is None) or size > 0): | |||
There was a problem hiding this comment.
| if buffering and ((size is None) or size > 0): | |
| if buffering and (size is None or size > 0): |
| # BufferedWriter will omit the write if size is 0 and the expedited SDO will not be sent | ||
| # Therefore, skip the BufferedWriter if size is known to be 0 |
There was a problem hiding this comment.
Please try to keep line lengths below 100 characters or even shorter. Especially for comments, longer lines are just annoying for those who user a narrow editor window ;-)
| # BufferedWriter will omit the write if size is 0 and the expedited SDO will not be sent | |
| # Therefore, skip the BufferedWriter if size is known to be 0 | |
| # BufferedWriter will omit the write if size is 0 and the expedited SDO will | |
| # not be sent. Therefore, skip the BufferedWriter if size is known to be 0. |
There was a problem hiding this comment.
Can I also suggest that the comment here is not very clear (I had to read it twice and read the code to get it). Perhaps have it say something more concise like "BufferedWriter will omit the write if size is 0. For size 0 use the raw stream to ensure the message is sent."
Previously, when attempting a write of
""to aVISIBLE_STRING, the expedited SDO request would not be sent and the transaction would timeout.I traced this down to the
BufferedWriternot issuing a write to the underlying stream when it has only been called withb"".To resolve this, if the transaction size is known to be
0, theraw_streamis now returned. I also added a unit test to cover this case.