-
-
Notifications
You must be signed in to change notification settings - Fork 13
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.
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
Add Cyphal/serial #128
Add Cyphal/serial #128
Conversation
Trying to test the example you gave here: https://github.com/maksimdrachov/test-serial-pr Coming up with some inconsistency, the output of running this script:
Is it because pycyphal/serial hasn't been updated yet to reflect this new part? (No issue for this can be found tho) Note: I think you forgot to mention which |
There might be an off-by-one error in your snippet. You can try and cross-validate it using these instructions: specification/specification/transport/serial/serial.tex Lines 158 to 164 in 4d80115
|
If it's not using pycyphal I'm not interested. |
xxd groups output by 16 bytes. Send it more data to see the full buffered
output.
…On Fri, May 12, 2023, 17:52 maksimdrachov ***@***.***> wrote:
Second example, using yakut:
[image: image]
<https://user-images.githubusercontent.com/39975120/238005522-48a1dd59-58b2-446a-914d-579bf0f8ff0b.png>
[image: image]
<https://user-images.githubusercontent.com/39975120/238005621-220f196f-03aa-43bb-a3e1-6ec1071e7a5e.png>
Doing something wrong here?
—
Reply to this email directly, view it on GitHub
<#128 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAZFIZE7AMF6PTAIFXFPXBDXFZFBRANCNFSM6AAAAAAX6S52RU>
.
You are receiving this because you were assigned.Message ID:
***@***.***>
|
The pointed-to element corresponds to the source node-ID, which is 1234 = 0x04d2. |
Ah, good catch! I changed the source node-ID to 4321 but failed to reflect this in the text. Fix incoming. |
If you could just push a commit updating the examples that would be dandy |
Ok! That's assuming the yakut one is correct? (It's using pycyphal under the hood?) I still wanna try with strictly pycyphal. |
Yes, Yakut uses PyCyphal under the hood. |
I think there's some issue with the current implementation of pycyphal. Resulting in a different result between yakut (which is using an older version) and pycyphal. See unit test in version 1.8: And compare with unit test in the latest version: I have updated the script here and even the length of the resulting frame is shorter (36 vs 42) |
Ok, I have taken a look at how the It calls Now in Two methods next to each other: I checked in |
They look okay to me.
This is because in Cyphal/UDP, sending is done via sessions by writing directly to the socket. |
Ok, we can discuss it next time we meet. I'm still a bit confused. |
…fy implementations.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Closes #99