Add messages: transponder navigation, modem ACKs

Breaking Changes

• Requires pyacomms >= 2.5

New Features

• Adds new messages that correspond to modem messages (published by acomms_driver_node):
  • NST ($CANST, Nav stats)
  • PDT ($SNPDT, Ping Digital Transponder Response)
  • PGT ($SNPGT, Ping Generic Transponder Response)
  • TTA ($SNTTA, Narrowband Transponder Ping Response)
  • ModemAck ($CAACK, Legacy ACK, and $CACDR, FDP Ack)
• acomms_driver_node publishes a std_msgs/Time message when $CAREV messages are received (sometimes used as a heartbeat)

Bugfixes

• Fixed the order of pip installs when doing gitlab-based tests.

I agree to the terms in the WHOI Contributor Agreement (CONTRIBUTING.md) in this repository.

added 49 commits

• f8de7b06...9cc64907 - 48 commits from branch master
• f1117d02 - merged v6.1.1 tag into dev/cordc/nav_msg_implementation branch

Compare with previous version

changed title from added 'verbose_msgs' flag for easy adding/creating/handling/recording Modem... to merged v6.1.1 tag into dev/cordc/nav_msg_implementation branch (PDT, NST, TTA)

changed title from merged v6.1.1 tag into dev/cordc/nav_msg_implementation branch (PDT, NST, TTA) to Add transponder navigation messages

changed the description

changed target branch from master to devel

We have a few things to do here:

• Reconcile this with the other transponder features. Do we actually want raw messages published?
• Rebase on devel and handle the acomms_sim merge

added 74 commits

• f1117d02...106fa826 - 71 commits from branch devel
• 4bf40ab8 - added NST, PDT, PGT, TTA ROS message output to handle incoming uModem SNNST,...
• 17917d65 - Merge branch 'dev/cordc/transponder_nav_msgs' into dev/cordc/nav_msg_implementation
• fab16107 - Merge remote-tracking branch 'origin/devel' into dev/cordc/nav_msg_implementation

Compare with previous version

Reconcile NAV Features

As part of our ongoing ACOMMS testing (transponder_survey, specifically), handling NAV msgs as separate ROS msgs provided a quick/easy way to document/record via ROSBAG-record node, without having to parse acomms_driver_node logs i.e. it was quicker/easier to hand off all data to the science team in a single .BAG

Also, a while back I was told the ping_transponder service was not quite robust enough (compared to the ping_modem service - I think it was based on edge case handling); so we do have a behavior node that does a CORDC [ping_transponder] routine separately, which takes advantage of the PDT, TTA, NST messages for analysis... If the ping_transponder service has been updated since ~summer 2022, then this might not be necessary.

I understand the question, which is why I wrapped the feature w/ a "verbose" flag if wanted/needed/desired - a poor attempt at thinking thru the problem, but timelines were tight.

Alternatively, maybe there is/could be a case where:

1. a user/node might be explicitly interested in NST msg since he/she/it/they explicitly set $CCCFG,nav.nst,1?
2. a backseat-node is passively analyzing the TTA msgs (SNTTA responses) [to eventually do something]?

Definitely a strong argument against publishing PDT, PGT messages...

Rebase

• rebased from branch:devel

ACOMMS_SIM Mege

• not sure what to to re: acomms_sim merge

Thanks for that background. I think that it makes sense to remove the rosparam that turns the messages on and off... you can always just not subscribe to them if you don't want them, and it reduces complexity. Can you push a change that does that?

One issue is that this does bump the pyacomms dependency, so it has to be a major version upgrade. I can't figure out how it's passing CI right now with the pypi version of pyacomms, which doesn't have some of those messages added. We'll need to bump the requirements.txt file, also.

Speaking of CI, I'm re-running pipelines now, since I think that some of them failed due to permissions issues, and not because anything is broken.

Let's try to get this in the v8 release.

changed milestone to %v8.0

requested review from @egallimore

It looks like each of these messages is being handled twice: once by the on_xxx callback, and also in a separate thread that is waiting on the the incoming message queue. I don't think we need both.. the on_xxx callbacks are fine, if our handlers are simple. The queue is more robust, but we should combine these into a single thread.

Either way, I'd like to consolidate some of the on_xxx callbacks. We could use functools.partial to simplify this, or just use object types.

e.g.

def on_casst(self, casst: Casst, msg: Message):

def on_snnst(self, snnst: NavStatistics, msg: Message) -> None:

etc... becomes (naming up for debate):

def on_modem_msg(self, parsed_object: Union[Casst, NavStatistics, ...], msg: Message) -> None:
    if isinstance(parsed_object, Casst):
        ......
    elif isinstance(.....

Then, all of the Micromodem object callbacks point to that single function.

changed milestone to %v8.1

I just committed the updated pyacomms requirement (daf65726), which is the only breaking change associated with the MR. This will allow us to add it as a feature release (I switched the milestone to %v8.1), which gives us some more time to work on cleanup.

just took care of this - I merged the latest devel branch and then combined CAREV, SST, NST, TTA, PDT, PGT to all use the single on_modem_msg function.

I considered moving the CST and XST messages as well, but I noted they were previously (what looked like specifically) added to their own threading queues and I didnt want to assume?

edit: I also removed the dependency on ~verbose_messaging but noted that it's been incorporated into the umodem logging utility

added 9 commits

• fab16107...b9feeb9a - 7 commits from branch devel
• 372ffb17 - Merge branch 'devel' into dev/cordc/nav_msg_implementation
• 47c32c6e - consolidated modem msgs to use 'on_modem_msg' cb; removed modem_XXX_handler...

Compare with previous version

changed milestone to %v9.0

I'm going to try to get this in the upcoming release (we're skipping v8.1 and going straight to v9.0).

added 3 commits

• 47c32c6e...ce1f7406 - 2 commits from branch devel
• 081caea6 - Merge branch 'devel' of https://git.whoi.edu/acomms/ros_acomms into...

Compare with previous version

excellent - no problem, sorry about the delay!