Skip to content
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

Final fixes for #70 (I think) #88

Merged
merged 7 commits into from
Jul 13, 2020

Conversation

thirtytwobits
Copy link
Member

Let me know if I missed any.

Let me know if I missed any.
Copy link
Member

@pavel-kirienko pavel-kirienko left a comment

Choose a reason for hiding this comment

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

3.7.5 (incl. 3.7.5.1) not updated.

Fun thing: 3.8.3.4 was updated in #70 correctly, and then it was un-updated again in #87. I'm not even sure what are we doing here. This is epic. Can you please update it again?

The examples in 4.2.3 require further updates. I propose we just drop them completely. Too much work.

@pavel-kirienko
Copy link
Member

The examples in 4.2.3 require further updates. I propose we just drop them completely. Too much work.

I looked at them again and they actually seem valuable because they explain the frame composition. Here is what needs updating.

Heartbeat

107D552A    00 00 00 00 E0 B1 01 E0
107D552A    01 00 00 00 E0 B1 01 E1
107D552A    02 00 00 00 E0 B1 01 E2
107D552A    03 00 00 00 E0 B1 01 E3

The above was obtained with the following command:

pyuavcan pub 4919.uavcan.primitive.String.1.0 '{}' --tr='CAN(can.media.socketcan.SocketCANMedia("vcan0", 64), 42)' --heartbeat-fields '{vendor_specific_status_code: 3471}' --count=4

Hello world

11133769   0C 00 48 65 6C 6C 6F 20 77 6F 72 6C 64 21 00 E0
11133769   0C 00 48 65 6C 6C 6F 20 77 6F 72 6C 64 21 00 E1
11133769   0C 00 48 65 6C 6C 6F 20 77 6F 72 6C 64 21 00 E2
11133769   0C 00 48 65 6C 6C 6F 20 77 6F 72 6C 64 21 00 E3

The command for the above:

pyuavcan pub 4919.uavcan.primitive.String.1.0 '{value: "Hello world!"}' --tr='CAN(can.media.socketcan.SocketCANMedia("vcan0", 64), None)' --count=4

Node info request

The node info request/response demo requires no modifications. For reference, it can be reproduced by running the PyUAVCAN demo application with export DEMO_INTERFACE_KIND=can and then pyuavcan call 42 uavcan.node.GetInfo.1.0 '{}' --tr='CAN(can.media.socketcan.SocketCANMedia("vcan0", 8), 123)'.

Natural array

1013373B   5C 00 00 01 02 03 04 05 06 07 08 09 0A 0B 0C 0D 0E 0F 10 11 12 13 14 15 16 17 18 19 1A 1B 1C 1D 1E 1F 20 21 22 23 24 25 26 27 28 29 2A 2B 2C 2D 2E 2F 30 31 32 33 34 35 36 37 38 39 3A 3B 3C A0
1013373B   3D 3E 3F 40 41 42 43 44 45 46 47 48 49 4A 4B 4C 4D 4E 4F 50 51 52 53 54 55 56 57 58 59 5A 5B 00 00 00 00 00 00 00 00 00 00 00 00 00 00 BC 19 40

(0x5C == 92)

The command for the above is:

pyuavcan pub 4919.uavcan.primitive.array.Natural8.1.0 '{value: [0,1,2,3,4,5,6,7,8,9,10,11,12,13,14,15,16,17,18,19,20,21,22,23,24,25,26,27,28,29,30,31,32,33,34,35,36,37,38,39,40,41,42,43,44,45,46,47,48,49,50,51,52,53,54,55,56,57,58,59,60,61,62,63,64,65,66,67,68,69,70,71,72,73,74,75,76,77,78,79,80,81,82,83,84,85,86,87,88,89,90,91]}' --tr='CAN(can.media.socketcan.SocketCANMedia("vcan0", 64), 59)'

Copy link
Member

@pavel-kirienko pavel-kirienko left a comment

Choose a reason for hiding this comment

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

3.7.5[.1], 3.8.3.4 still need updating. Do you want to merge this one and update them later or do you prefer to do everything in this PR?

Also updating latex tooling to fix new requirements for port forwarding
in the docker container.
@thirtytwobits
Copy link
Member Author

3.7.5[.1], 3.8.3.4 still need updating. Do you want to merge this one and update them later or do you prefer to do everything in this PR?

What's left to do in 3.8.3.4?

Copy link
Member

@pavel-kirienko pavel-kirienko left a comment

Choose a reason for hiding this comment

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

Great.

3.7.5.1 is fixed (with one remaining issue) but 3.7.5 requires updating (the example in the blue box; although it can be simplified if desired).

3.8.3.4, as I wrote above, was accidentally changed twice. If you look at the boolean flags you will see that they need to be ordered the other way around because we changed the bit order; here's the correct order:

<...>
uint8 FLAG_COOLING_SYSTEM_B_ACTIVE = 1
uint8 FLAG_COOLING_SYSTEM_A_ACTIVE = 2
uint8 FLAG_PSU_MALFUNCTION = 32
<...>
<...>
bool flag_cooling_system_a_active  # bit 0
bool flag_cooling_system_b_active  # bit 1
bool flag_psu_malfunction          # bit 2
<...>

@thirtytwobits
Copy link
Member Author

here's the correct order:

<...>
uint8 FLAG_COOLING_SYSTEM_B_ACTIVE = 1
uint8 FLAG_COOLING_SYSTEM_A_ACTIVE = 2
uint8 FLAG_PSU_MALFUNCTION = 32
<...>
<...>
bool flag_cooling_system_a_active  # bit 0
bool flag_cooling_system_b_active  # bit 1
bool flag_psu_malfunction          # bit 2
<...>

You'll note that you've given B_ACTIVE = 1 and A_ACTIVE = 2 so the correct order would be

<...>
bool flag_cooling_system_b_active  # bit 0
bool flag_cooling_system_a_active  # bit 1
bool flag_psu_malfunction          # bit 2
<...>

unless you didn't mean for B to be 1 and A to be 2? I found that choice curious myself but I suspected there was some reason I didn't get.

@pavel-kirienko
Copy link
Member

I found that choice curious myself but I suspected there was some reason I didn't get.

You are right. The inverted order made sense before we switched the bit order because you will see that after we replaced the bit mask with separate bit fields the order is such that A comes before B. Now, with the little-endian bit order, it's best to swap them, although I wouldn't say that it's critically important.

@pavel-kirienko pavel-kirienko merged commit afed99b into OpenCyphal:master Jul 13, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants