10000 Improve slcan.py by mikisama · Pull Request #1490 · hardbyte/python-can · GitHub
[go: up one dir, main page]
Skip to content

Improve slcan.py #1490

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 21 commits into from
Feb 4, 2023
Merged

Improve slcan.py #1490

Changes from 1 commit
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
2c9e1e6
Improve slcan.py
mikisama Jan 17, 2023
f95ca7d
Merge branch 'hardbyte:develop' into develop
mikisama Jan 27, 2023
7fa9f5c
use `read_all` to read out serial port data.
mikisama Jan 27, 2023
c581cc9
fix when the `timeout` parameter is 0, it cannot enter the while loop.
mikisama Jan 27, 2023
8722a11
For performance reason, revert to using `read` to read serial data.
mikisama Jan 28, 2023
fa62a63
add `size=1` and renamed `new_data` to `new_byte`
mikisama Jan 29, 2023
73ce3dd
fix the issue of returning `None` before receiving
mikisama Jan 29, 2023
3eb80b9
Due to the simplification of the control flow,
mikisama Jan 29, 2023
2bbee49
Revert "Due to the simplification of the control flow,"
mikisama Jan 29, 2023
1635381
Simplify the control flow for finding the end of a SLCAN message.
mikisama Jan 29, 2023
42db106
improve the `timeout` handling of slcan.py
mikisama Jan 31, 2023
6f994cc
Merge branch 'hardbyte:develop' into develop
mikisama Jan 31, 2023
3c8a9e3
Change the plain format calls to f-strings.
mikisama Jan 31, 2023
ce3b9a4
using `bytearray.fromhex` to parse the frame's data.
mikisama Jan 31, 2023
2c22b4e
change `del self._buffer[:]` to `self._buffer.clear` since it's more …
mikisama Jan 31, 2023
09e1248
Simplify the timeout handling
mikisama Jan 31, 2023
e3a388b
fix the issue when the returned string is `None`.
mikisama Jan 31, 2023
370a2ff
using `pyserial.reset_input_buffer` to discard
mikisama Jan 31, 2023
68c9c71
fix failing PyPy test
mikisama Feb 1, 2023
cec01d7
fix failing PyPy test
mikisama Feb 1, 2023
6c3b13a
improve the comments
mikisama Feb 1, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
fix when the timeout parameter is 0, it cannot enter the while loop.
  • Loading branch information
@mikisama
mikisama committed Jan 27, 2023
commit c581cc977a41ffab3b041e7a06a352b891e9eb0f
9 changes: 6 additions & 3 deletions can/interfaces/slcan.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -153,13 +153,16 @@ def _read(self, timeout: Optional[float]) -> Optional[str]:
_timeout = serial.Timeout(timeout)

with error_check("Could not read from serial device"):
while not _timeout.expired():
while True:
new_data = self.serialPortOrig.read_all()
if new_data:
self._buffer.extend(new_data)
else:
time.sleep(0.001)
continue
if _timeout.expired():
Copy link
Contributor Author
@mikisama mikisama Jan 29, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we really need _timeout to handle timeout?

The timeout can be determined when pyserial.read returns b''.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The timeout of pyserial.read is reset on every read call, so it might add up. The timeout argument of the user should be respected, _timeout takes care of that.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the timeout's add up should be ignored.
For python on windows, the timer's resolution is about 16ms, obviously it is not very accurate.
And I think the timeout add up is not as big as the impact of timer jitter.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That depends on the timeout value, It might be seconds, or minutes, or hours, i don't know what users might be doing.

break
else:
time.sleep(0.001)
continue

for terminator in (self._ERROR, self._OK):
if terminator in self._buffer:
Expand Down
0