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

Fix telemetry #52

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

Fix telemetry #52

wants to merge 2 commits into from

Conversation

uellenberg
Copy link
Collaborator

Based on #51, and requires that to be merged before this. Reviewer: ignore changes to roslib code (I would suggest just reviewing the second commit directly).

This fixes telemetry. I tested it on the rover and was able to generate a CSV file as well as view the data in the webpage.

This creates a cache of topics such that creating
new subscribers and publishers don't incur extra
network requests.
If no subscribers are listening to a topic, then
messages won't be sent to it.
@adamseth2
Copy link
Collaborator

Im assuming it is still bugged where the values are not updating with the UI. Lmk when that is resolved

@uellenberg
Copy link
Collaborator Author

Im assuming it is still bugged where the values are not updating with the UI. Lmk when that is resolved

@adamseth2 I'm not sure what you mean. When I tested, values were updating in the UI.

@adamseth2
Copy link
Collaborator

Im assuming it is still bugged where the values are not updating with the UI. Lmk when that is resolved

@adamseth2 I'm not sure what you mean. When I tested, values were updating in the UI.

I swear you showed me it was bugging where the telemetry values were not updating, but maybe I am remembering it wrong.

Also is this what it is supposed to look like when not connected with the rover? Aka what the select does?

image

Have never tested the feature out but a potential fix for the Update Delay in ms that can be done on the mission control side is roslib can specific throttle_rate for topics, but unsure if it actually saves bandwidth or not.

@adamseth2
Copy link
Collaborator

Ahh finally got what mission_control_updater does! This has always been an idea flowing around if having an interface between the mission control and the rover is what we should do (mission control shouldn't typically directly sub to the topics used in the rover but specialized topics meant for the mission control like in mission_control_updater).

I think we should expand upon mission_control_updated to include other telemetry data or other info like temperature as this data is not essentially to be updated right away and having it sent at once in a big package is probably better than multiple smaller ones.

Basically, I think moving the mission_control_updated to a store would be helpful as other modules/ pages might I.e. the home page might have a more visualized version and include the most important data of the rover or the arm page might want the arm data also.

Good work and appreciate the work you have done!

@uellenberg
Copy link
Collaborator Author

Im assuming it is still bugged where the values are not updating with the UI. Lmk when that is resolved
@adamseth2 I'm not sure what you mean. When I tested, values were updating in the UI.

I swear you showed me it was bugging where the telemetry values were not updating, but maybe I am remembering it wrong.

Also is this what it is supposed to look like when not connected with the rover? Aka what the select does?

image

Have never tested the feature out but a potential fix for the Update Delay in ms that can be done on the mission control side is roslib can specific throttle_rate for topics, but unsure if it actually saves bandwidth or not.

Yeah, as soon as it connects it should pull down the available data.

@uellenberg
Copy link
Collaborator Author

Ahh finally got what mission_control_updater does! This has always been an idea flowing around if having an interface between the mission control and the rover is what we should do (mission control shouldn't typically directly sub to the topics used in the rover but specialized topics meant for the mission control like in mission_control_updater).

I think we should expand upon mission_control_updated to include other telemetry data or other info like temperature as this data is not essentially to be updated right away and having it sent at once in a big package is probably better than multiple smaller ones.

Basically, I think moving the mission_control_updated to a store would be helpful as other modules/ pages might I.e. the home page might have a more visualized version and include the most important data of the rover or the arm page might want the arm data also.

Good work and appreciate the work you have done!

I'm not actually sure what mission_control_updater is for. I just noticed that it isn't used and it gives exactly the same data that telemetry wants, so I used it here.

I wouldn't make it a store, since then we lose the ability to detect when no code is making use of it. I do think making it smaller would be good, however (i.e., having multiple topics).

@adamseth2
Copy link
Collaborator

Im assuming it is still bugged where the values are not updating with the UI. Lmk when that is resolved
@adamseth2 I'm not sure what you mean. When I tested, values were updating in the UI.

I swear you showed me it was bugging where the telemetry values were not updating, but maybe I am remembering it wrong.
Also is this what it is supposed to look like when not connected with the rover? Aka what the select does?
image
Have never tested the feature out but a potential fix for the Update Delay in ms that can be done on the mission control side is roslib can specific throttle_rate for topics, but unsure if it actually saves bandwidth or not.

Yeah, as soon as it connects it should pull down the available data.

I would then hide select and start record and just say "not detected" if its not connected. This would make it clear that this page is not working properly yet and help with debugging as sometimes not motors are working correctly.

@adamseth2
Copy link
Collaborator

In the context of Aaron and Alan needs the telemetry data for visualization and the arm , it makes sense that it should be easily shared. Making it a store will not make it lose the ability to stop it because we can use lifecycle methods to make the subscribers stop similarly done in exampleComponent.vue.

Here are a couple of things I noticed:

  1. We can safely assume MoteusMotorState type is not going to change, so we can kind of hard code it, meaning make a typescript type of https://github.com/TrickfireRobotics/urc-2023/blame/main/src/lib/moteus_motor_state.py#L14 , so we don't have to first parse the keys .
  2. When you enter the telemetry page, the driver expects (also have no time too) to see all the data without having to checkmark or select anything.
  3. The parsing can be done in the callback in the subscriber and make it so that the subscriber holds an array of moteusmotorstate.

Future things after the PR:

  1. mission_control_updater will change so that it publishes the state of the motors separately. (Not sure if going to use JSON or have a defined messageType yet)
  2. Im currently working on making the auto unsub better

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.

2 participants