SoundMetrics / aris-integration-sdk

SDK for building custom controllers for the ARIS sonar.
MIT License
5 stars 2 forks source link

Header Updates #75

Closed FBaralli closed 6 years ago

FBaralli commented 6 years ago

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:

Thanks Francesco

curtnichols commented 6 years ago

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:

If you don't mind my asking:

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 commented 6 years ago

Curt,

thank you for the quick feedback. My question about moving the ArisFrameHeaderUpdateMsg to protobuf was mostly related to the fact that the struct serialization comes for free under protobuf. I'm developing code in C++ with C++14 extensions and I use the libprotobuf that is available as standard under Ubuntu 16.04 (I believe it is version 1.2.1). At the moment I'm having troubles sending the struct as UDP message (using boost asio) because it doesn't seem to be accepted by the camera. Is there a flag that informs if the header update was accepted or not? Do you have a code snippet for that?

Best Regards Francesco

Il giorno mer 21 mar 2018 alle ore 18:19 Curt Nichols < notifications@github.com> ha scritto:

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

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/SoundMetrics/aris-integration-sdk/issues/75#issuecomment-375025213, or mute the thread https://github.com/notifications/unsubscribe-auth/AIIotVfs1MJbPJ1y_S0HA-Z08HYo1xvfks5tgouygaJpZM4SzUbl .

curtnichols commented 6 years ago

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 commented 6 years ago

Curt, I'll do some more tests tomorrow (I'm writing from Italy and we are currently doing some at sea tests) and get back to you with some feedback. Would you prefer to keep this thread open on GitHub or rather switch to a regular email?

Regards Francesco

Il giorno mer 21 mar 2018 alle ore 19:35 Curt Nichols < notifications@github.com> ha scritto:

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: image] https://user-images.githubusercontent.com/2353284/37729863-666e34f8-2cfb-11e8-8fd9-0044656e09dc.png

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/SoundMetrics/aris-integration-sdk/issues/75#issuecomment-375051706, or mute the thread https://github.com/notifications/unsubscribe-auth/AIIotboatlT1rQ-d-IkXG9M9_LJcoEySks5tgp2PgaJpZM4SzUbl .

curtnichols commented 6 years ago

I think discussing here is fine.

Curt

curtnichols commented 6 years ago

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.

FBaralli commented 6 years ago

Hi Curt,

nPktNum=0 seems to have solved the problem.

Thanks a lot.

Regards Francesco

Il mer 21 mar 2018, 23:01 Curt Nichols notifications@github.com ha scritto:

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.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/SoundMetrics/aris-integration-sdk/issues/75#issuecomment-375110448, or mute the thread https://github.com/notifications/unsubscribe-auth/AIIotSNfmQDupRSDpq4htWsV4eiM-UzBks5tgs3FgaJpZM4SzUbl .

curtnichols commented 6 years ago

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