Cobs #126
Cobs #126
Conversation
There was a problem hiding this comment.
At the whole pyuavcan level there are some dsdl test failures that are probably setup-related (PYTHONASYNCIODEBUG)
You should be able to fix that by exporting PYTHONASYNCIODEBUG=1, or just by running test.sh.
Some of the docs make references to escaping which may need reviewing. For example, the docs for the method SerialFrame.compile_into talk about escaping, is it still valid?
The docs would benefit if you added a link to https://forum.uavcan.org/t/uavcan-serial-issues-with-dma-friendliness-and-bandwidth-overhead/846 because that thread provides an exhaustive explanation of why COBS has been chosen.
| @@ -64,13 +64,6 @@ def _process_byte(self, b: int, timestamp: pyuavcan.transport.Timestamp) -> None | |||
| return | |||
|
|
|||
| # Unescaping is done only if we're inside a frame currently. | |||
pyuavcan/transport/serial/_frame.py
Outdated
|
|
||
| out_buffer[next_byte_index] = self.FRAME_DELIMITER_BYTE | ||
| next_byte_index += 1 | ||
|
|
||
| assert (next_byte_index - 2) >= (len(header) + len(self.payload) + len(payload_crc_bytes)) | ||
| return memoryview(out_buffer)[:next_byte_index] | ||
|
|
||
| @staticmethod | ||
| def calc_cobs_size(payload_size_bytes: int) -> int: | ||
| return int(math.ceil(payload_size_bytes * 255.0 / 254.0)) |
There was a problem hiding this comment.
It is better to avoid floating point when dealing with discrete entities like bytes for simplicity and to provide cleaner implementation examples for people who will be re-implementing this logic for embedded systems.
| return int(math.ceil(payload_size_bytes * 255.0 / 254.0)) | |
| return (payload_size_bytes * 255 + 253) // 254 |
This change is equivalent which can be verified:
>>> set(int(math.ceil(payload_size_bytes * 255.0 / 254.0)) - ((payload_size_bytes * 255 + 253) // 254) for payload_size_bytes in range(10000000))
{0}The math import is no longer needed.
|
The docs for the method SerialFrame.compile_into reads valid to me, considering COBS is also an escaping of sorts. |
There was a problem hiding this comment.
There is one issue found by MyPy. Please accept my suggestion, then we should be good to go. The other issues from MyPy are caused by the fact that its version number is not yet frozen (because the library is still being actively developed); a new one was released which found extra issues in unrelated code. I should freeze it soon.
pyuavcan/transport/serial/_frame.py
Outdated
| import dataclasses | ||
| import pyuavcan | ||
| from cobs import cobs |
There was a problem hiding this comment.
This is to suppress the MyPy warning about the type hints being unavailable.
| from cobs import cobs | |
| from cobs import cobs # type: ignore |
There was a problem hiding this comment.
yes, just committed this fix
Pull Request Test Coverage Report for Build 35677795
💛 - Coveralls |
Switched from brute-force escaping to COBS for serial protocol serialization