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

Header Updates #75

Closed
FBaralli opened this issue Mar 21, 2018 · 8 comments
Closed

Header Updates #75

FBaralli opened this issue Mar 21, 2018 · 8 comments
Assignees
Labels

Comments

@FBaralli
Copy link

Hello!

I'm currently working on the integration of an ARIS3000 into an unmanned platform. I've been quite successful in re-using part of the code in this repo; however I'm now struggling trying to send telemetry data in the form of HeaderUpdates. Hence few questions about that:

  • is the conversion from struct ArisFrameHeaderUpdateMsg to protobuf expected to happen at some point in the future?
  • is it required to transmit the struct in network order?

Thanks
Francesco

@curtnichols curtnichols self-assigned this Mar 21, 2018
@curtnichols
Copy link
Member

Hi Francesco,

Glad to hear things are largely going well. There is currently no plan to move ArisFrameHeaderUpdateMsg to a protobuf style message, though I appreciate that would be consistent with the ARIS 2 protocols.

The ARIS header update message is a bit of a legacy part:

  • Unlike the protobuf-based messages, there is no length prefix. It's UDP-based, so we know the size of the message.
  • It is a flat "C struct" style message with no consideration for network order, it assumes Little Endian (Intel) convention. (This is contrary to the later protobuf-based protocols, which require network order for the length prefix.)
  • The struct must be packed (aligned) to 1-byte boundaries.

If you don't mind my asking:

  • What programming language(s) are you using for the ARIS integration?
  • What protobuf library & version are you using? (A future update will move our .proto files to the protobuf syntax version 3.)

I will open an issue in one of our private repos to discuss the possibility of adding the ability to use protobuf for the header update.

Curt

@FBaralli
Copy link
Author

FBaralli commented Mar 21, 2018 via email

@curtnichols
Copy link
Member

curtnichols commented Mar 21, 2018

Francesco, here's what I can add:

  1. Presently there's not a flag indicating success.
  2. An incorrect size is not logged. (Unexpected network traffic could fill our logs.)
  3. sizeof(ArisHeaderUpdateMsg) should be 168 (https://godbolt.org/g/CRBcJ1).

Problems with these items should appear in the sonar logs (these problem log entries would be prefixed [Bluefin]):

  1. header.nCommand must be ArisHeaderUpdate::C_DATA (0xa502).
  2. header.nPktNum must be non- zero.
  3. header.nPktType must have bit ArisHeaderUpdate::UPDATE_HEADER (0x0040) set to 1.

Also,

  1. update.m_nUpdateFlagsmust have the bit flag set for each field you're updating; e.g., ArisHeaderUpdate::UPDATE_ROLL (0x00000020) must be set to update Roll.

If the above conditions are met the header update is accepted.

Regarding logs, Wireshark is useful for watching live logs. The sonar relays many log messages to the controlling host on UDP port 514.

image

@FBaralli
Copy link
Author

FBaralli commented Mar 21, 2018 via email

@curtnichols
Copy link
Member

I think discussing here is fine.

Curt

@curtnichols
Copy link
Member

One correction to the above:

  1. header.nPktNum must be zero.

Unfortunately this made it into the sample code in ArisHeaderUpdate.h, which I'll correct on master.

curtnichols added a commit that referenced this issue Mar 21, 2018
Merging to master; unfortunately the build configuration isn't correct for master right now, so it'll be broken for a bit.
curtnichols added a commit that referenced this issue Mar 22, 2018
* Fix an issue found in #75 and add the send-header-update sample (#76)

Merging to master; unfortunately the build configuration isn't correct for master right now, so it'll be broken for a bit.

* Correct the comment for nPktNum
@FBaralli
Copy link
Author

FBaralli commented Mar 22, 2018 via email

@curtnichols
Copy link
Member

Glad to hear it, I've fixed the header comments and sample function.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants