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

Update Telemetry to Use Services Rather than Publishers #72

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

VladimirKupryukhin
Copy link
Contributor

What is this

The old telemetry code uses publishes that always publish data to the ROS network every 4ms. This is bad since we potentially flood the web socket with data that potentially is not being used. Instead, this PR changes it into a service based system. The mission control asks for data, and the rover gives it

What was changed

  • Added a services into the RobotInfo under "get_moteus_motor_state"
  • Unrelated update to the container_launch.sh to properly name the image as trickfireimage rather than trickfirerobot.

Copy link
Member

@loukylor loukylor left a comment

Choose a reason for hiding this comment

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

Does this work in its current state? I'm like 90% sure since there are multiple RobotInfo instances floating around, there will be multiple handlers for the service, and therefore multiple nodes trying to respond to a single request.

@VladimirKupryukhin
Copy link
Contributor Author

Yes, you are right. Check out the "Note" found here: https://docs.ros.org/en/humble/Concepts/Basic/About-Services.html
Since technically all the RobotInfo data is the same everywhere, it does not matter which RobotInfo responds to the request. But that is just bad code design by me.

I will make a new node, perhaps called the MissionControlRobotInfo that will contain an instance of RobotInfo and create that service to handle the requests.

@loukylor loukylor mentioned this pull request Oct 21, 2024
@uellenberg
Copy link
Collaborator

Note that services will likely increase the latency due to requiring a round trip. We may get more mileage by using a heartbeat-like system to tell the rover that we want to receive the data (and maybe also specifying the update rate). The latency is still there, it's just being moved to the end so that when we stop looking at it, it takes a bit of time before it stops sending it (which isn't a huge issue, in my opinion).

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.

4 participants