-
Notifications
You must be signed in to change notification settings - Fork 316
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
Comms bridge #756
Comms bridge #756
Conversation
…at location would work
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.
Thanks for sharing this! Great to see how far it's coming along.
Comments mostly trivial.
I had a hard time understanding how to estimate the memory usage. The 128K per content message buffer allocation could potentially get bad quickly if we aren't careful. But maybe it doesn't matter for ISAAC in the near term if we don't subscribe to high-rate messages.
…obee into comms_bridge
5b6f8c0
to
3897858
Compare
Added an out topic to link entries so one can specify an out topic name if the8y want. Also added code so that all topics don't go to all robots.
The comms bridge was changed to start the dds subscribers and publishers either on start up if specified in the comms bridge configuration file or when the executive calls the enable comms service. This matches how the current astrobee to astrobee bridge works.
aa51b37
to
0d1c82b
Compare
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.
Tested latest version in the lab
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.
The current configuration for the bridge is not exactly what other users would need. Usually, they need /gnc/ekf both ways. This is not a big deal though since it can be configured.
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.
Thanks! I'm really excited about this coming together and hope other Astrobee users will get some benefit. My comments are minor and mostly about docs.
astrobee/config/communications/dds_generic_comms/NDDS_DISCOVERY_PEERS
Outdated
Show resolved
Hide resolved
@@ -71,11 +71,11 @@ echo " Analysing python code style with 'isort'." | |||
# could happen for example if the .isort.cfg src_paths list gets out | |||
# of date. | |||
|
|||
if $(isort . --extend-skip cmake --profile black --diff --check-only --quiet >/dev/null); then | |||
if $(isort . --extend-skip cmake,submodules --profile black --diff --check-only --quiet >/dev/null); then |
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.
Interesting. Should we propagate this change to the isaac
repo? Not sure why the relevant commands are different.
https://github.com/nasa/isaac/blob/develop/scripts/git/pre-commit.linter_python#L75-L79
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.
(To answer my own question: "Yes." nasa/isaac#123 )
This is the comms bridge that allows any ros message to be sent between robots without having to specifically add ros to dds and vice versa code.