openpilot icon indicating copy to clipboard operation
openpilot copied to clipboard

Adding foxglove support

Open savojovic opened this issue 1 year ago • 11 comments

In this approach, route logs were initially converted from Python dictionaries to JSON format, following creation of mcap file, incorporating data formatted in JSON. For each event, a JSON schema was generated. Support is added for all properties that are not in a list. Foxglove creates one graph for all values in an array instead of one graph for every element in an array. In addressing this matter, an example workaround has been implemented specifically for 'deviceState.cpuTempC' (supported by a screenshot, plotjuggler, and Foxglove). Screenshot from 2024-03-18 15-06-24

Known functionalities to be implemented:

  • S̶u̶p̶p̶o̶r̶t̶ f̶o̶r̶ e̶v̶e̶r̶y̶ p̶r̶o̶p̶e̶r̶t̶y̶ t̶h̶a̶t̶ i̶s̶ i̶n̶ a̶ l̶i̶s̶t̶ (̶d̶o̶n̶e̶ f̶o̶r̶ d̶e̶v̶i̶c̶e̶S̶t̶a̶t̶e̶.c̶p̶u̶T̶e̶m̶p̶C̶)̶
  • O̶v̶e̶r̶h̶e̶a̶d̶ o̶f̶ c̶a̶l̶l̶i̶n̶g̶/c̶o̶n̶v̶e̶r̶t̶i̶n̶g̶ j̶s̶o̶n̶ c̶o̶u̶l̶d̶ b̶e̶ r̶e̶d̶u̶c̶e̶d̶
  • C̶h̶e̶c̶k̶ a̶n̶d̶ i̶n̶s̶t̶a̶l̶l̶ f̶o̶x̶g̶l̶o̶v̶e̶
  • Add cam support?
  • A̶d̶j̶u̶s̶t̶ m̶a̶p̶ a̶n̶d̶ i̶m̶a̶g̶e̶ r̶e̶n̶d̶e̶r̶i̶n̶g̶ t̶o̶ a̶ f̶o̶r̶m̶a̶t̶ t̶h̶a̶t̶ f̶o̶x̶g̶l̶o̶v̶e̶ u̶n̶d̶e̶r̶s̶t̶a̶n̶d̶s̶

#30272

savojovic avatar Mar 18 '24 14:03 savojovic

Thanks for contributing to openpilot! In order for us to review your PR as quickly as possible, check the following:

  • Convert your PR to a draft unless it's ready to review
  • Read the contributing docs
  • Before marking as "ready for review", ensure:
    • the goal is clearly stated in the description
    • all the tests are passing
    • the change is something we merge
    • include a route or your device' dongle ID if relevant

github-actions[bot] avatar Mar 18 '24 14:03 github-actions[bot]

Added thumbnail support. Screenshot from 2024-03-19 15-18-55

adeebshihadeh, could you check if this has any potential so far.

savojovic avatar Mar 19 '24 14:03 savojovic

Map and thumbnail example

up

savojovic avatar Mar 21 '24 12:03 savojovic

Just a nit but could you change it to "two space indentation" to match the rest of openpilot?

~I think the schemas/*.json should be .gitignore-d~ How are the schemas/*.json generated? I don't think they should have to be manually updated (or have that many lines checked in to openpilot)

incognitojam avatar Mar 21 '24 15:03 incognitojam

You are right, dynamically creating schemas would be a better approach. I will try to find the most effective method for mapping existing capnp schemas to JSON schemas. The indentation will be fixed as well.

savojovic avatar Mar 21 '24 21:03 savojovic

It seems that the default foxglove map cannot focus on the route automatically. It seems like the solution could be implemented using a custom map option and OpenStreetMap.

savojovic avatar Mar 30 '24 16:03 savojovic

Nice work! Is the script here enough to create the map and thumbnail example above?

I'm a little hesitant to invest any more in Foxglove since they stopped development on it (see their blog). Of course, if this works, we'll still pay out the bounty.

adeebshihadeh avatar Mar 31 '24 00:03 adeebshihadeh

That's too bad. It has a really nice GUI and a lot of functionalities. The current script supports maps and thumbnails from the example above. As I mentioned earlier, the map panel seems a bit buggy (on their side). It could be improved by using the custom maps option that they provide. I was not sure if the current solution for maps is good or not, so I didn't invest much time in it.

Screenshot captured using script from the latest git commit

here1

savojovic avatar Mar 31 '24 08:03 savojovic

Hey, @adeebshihadeh did you decide whether to go with foxglove. Do I continue to work on this PR, if so in which direction should I go? Are there some features/parts that I should focus on?

savojovic avatar Apr 03 '24 08:04 savojovic

Hey, @adeebshihadeh did you decide whether to go with foxglove. Do I continue to work on this PR, if so in which direction should I go? Are there some features/parts that I should focus on?

Unfortunately, I don't think I'll have time this week to give a proper review or investigate where we should go with this project. I propose the following:

  • let's leave this PR alone for now. if it works as well as our PlotJuggler stuff, we'll pay out the bounty. if it only kinda works, we'll pay out a partial bounty
  • https://www.rerun.io/ seems to be the new thing. I haven't messed with it yet, but I put up a new bounty and locked it to you https://github.com/commaai/openpilot/issues/32096

If you have any more questions, let's discuss over in #dev-openpilot-private on Discord. I think the community will have some good feedback here too.

adeebshihadeh avatar Apr 03 '24 16:04 adeebshihadeh

Hey, @adeebshihadeh did you decide whether to go with foxglove. Do I continue to work on this PR, if so in which direction should I go? Are there some features/parts that I should focus on?

Unfortunately, I don't think I'll have time this week to give a proper review or investigate where we should go with this project. I propose the following:

  • let's leave this PR alone for now. if it works as well as our PlotJuggler stuff, we'll pay out the bounty. if it only kinda works, we'll pay out a partial bounty
  • https://www.rerun.io/ seems to be the new thing. I haven't messed with it yet, but I put up a new bounty and locked it to you [$300 bounty] rerun.io support #32096

If you have any more questions, let's discuss over in #dev-openpilot-private on Discord. I think the community will have some good feedback here too.

Thanks for the feedback. Sounds good to me, I will check the rerun task.

savojovic avatar Apr 04 '24 07:04 savojovic

@jnewb1 can you review this? if it's useful, let's merge and pay out

adeebshihadeh avatar Apr 04 '24 17:04 adeebshihadeh

The biggest delays happen when downloading logs using LogReader and writing messages into mcap.Writer. This is because we're opening files containing log messages over and over again in a loop, which eats up time.

One quick way to tackle this is by doing IO operations in parallel and turning off compression in the mcap.Writer object. Another option is to write everything into a single big file, which could get rid of the overhead entirely. But doing that would mean we'd need to figure out how to navigate through that big file efficiently. @jnewb1

savojovic avatar Apr 05 '24 11:04 savojovic

Merged. We'll pay out the full bounty for this one.

Let's see how rerun.io works out. If it's good, we'll go with that and delete this. If it's not good for some reason, we can setup some more bounties to make this fast and as good as PlotJuggler.

adeebshihadeh avatar Apr 05 '24 18:04 adeebshihadeh

This creates hundreds of thousands of files in tools/foxglove/rlogs loading the demo route, can we fix that or does foxglove expect each timestep as a file? Unrelated, but it caused PyCharm to hang indexing them :/

256,706 items, totalling 475.2 MB

sshane avatar Apr 05 '24 22:04 sshane

This creates hundreds of thousands of files in tools/foxglove/rlogs loading the demo route, can we fix that or does foxglove expect each timestep as a file? Unrelated, but it caused PyCharm to hang indexing them :/

256,706 items, totalling 475.2 MB

This is just a POC - we don't want to spend any time on it until we find out whether we want this or rerun.io

adeebshihadeh avatar Apr 05 '24 22:04 adeebshihadeh

This creates hundreds of thousands of files in tools/foxglove/rlogs loading the demo route, can we fix that or does foxglove expect each timestep as a file? Unrelated, but it caused PyCharm to hang indexing them :/

256,706 items, totalling 475.2 MB

Actually, I believe that the 'rlogs/' and 'schemas/' directories could be deleted after the '.mcap' file has been created. But I haven't tested it yet.

savojovic avatar Apr 05 '24 22:04 savojovic