PX4 / PX4-Autopilot

PX4 Autopilot Software
https://px4.io
BSD 3-Clause "New" or "Revised" License
8.2k stars 13.37k forks source link

Crawl PX4 codebase and generate pub/sub graph #6189

Closed LorenzMeier closed 6 years ago

LorenzMeier commented 7 years ago

@CarloWood Would you be interested in helping us out with a Python script (there are a few like the px_generate_params.py and px_generate_airframes.py) that:

This would be extremely helpful as it would show developers how the system relates. Eventually I'd like to grow this into a website that links to Doxygen for each module.

LorenzMeier commented 7 years ago

Can then probably use d3 to eventually display: https://github.com/d3/d3/wiki/Gallery

stephanbro commented 7 years ago

I gave this a whirl for fun, but the graph is incredibly complex (I used graphviz and the px4fmu-v2 config). test.gv.pdf

Maybe it makes more sense to split things up by topic? Does d3 handle complex graphs like this well? SigmaJS could be another option.

dagar commented 7 years ago

Ha, thanks @stephanbro. That's more spaghetti than I was expecting. Anyone familiar with good tools for displaying graphs interactively? Optionally selecting modules and topics.

dagar commented 7 years ago

Just a few thoughts from looking at it.

If that's still way too much then separate graphs for MC, FW, VTOL could help.

Ultimately it would be nice to have everything optionally browsable, but stripping it down to the core in order to make it comprehensible would really people understand the architecture.

LorenzMeier commented 7 years ago

@stephanbro Can you send a PR with the script so we can iterate together on it?

dagar commented 7 years ago

Random thought - would this be easier/better to do at runtime? Have some optional code (in uorb?) so that each publication records the module, topic, count, and likewise for each subscription.

Then we can generate the graph for an actual vehicle/flight and color/size each edge based on number of publications.

stephanbro commented 7 years ago

@dagar I went with you suggestion and removed the mavlink, sdlog2, and logger modules and things look a lot cleaner (though not all the way there yet). Here's an example of that: pub_sub_nomavlink.pdf

And here is an example removing the less common drivers you suggested and load_mon: pub_sub_nomavlink_noload_mon.pdf

stephanbro commented 7 years ago

I also did a MC only graph, but even that feels a bit too cluttered. You can see it here: pub_sub_mc.pdf

I'm starting to think that the dynamic viewer using JS is going to be the best option.

dagar commented 7 years ago

Nice! It's getting close to usable. I think it depends what the ultimate purpose of this would be. A dynamic viewer sounds amazing, but realistically would a developer even somewhat familiar with the codebase bother using it when they could just grep?

I personally think the majority of the value is giving potential new developers a gentle and accurate introduction to the architecture that we can easily keep updated. I think we could even get there with a few more tweaks of this existing script.

Next

stephanbro commented 7 years ago

Here is a version using the module level and dropping the additional items you suggested: pub_sub_module.pdf

There are some known issues with how I'm grabbing the topic which can be see in this line: topic = re.search('ORB_ID\((.+?)\)', line). Right now there are some items, such as the sensor data, which don't follow this method for subscription, so they don't get picked up by the script.

As for combining all edges, it might be do-able. I need to do some digging in graphviz to see what is possible.

stephanbro commented 7 years ago

I've done some small tweaks to also look for the orb_copy lines instead of just orb_subscribe, which fixed the issue with the sensors missing connection (possibly other items as well). There are some outlying issues as I'm still relying on the regex in the previous comment.

I was able to add some extra loops and condense the number of edges as you suggested: pub_sub_combined_edges.pdf

Here is re-adding the mavlink, logger, and sdlog2: pub_sub_combined_edges_logger.pdf

dagar commented 7 years ago

So I actually quite like the last two, but I would guess it still looks like spaghetti to someone new? What do you think?

If you want to update it in #6368 I could get it running on a CI system and uploading the output to s3. Then we could continue tweaking and try different versions for MC, FW, VTOL, and perhaps different levels of detail.

Nice work!

dagar commented 7 years ago

Another random idea to explore later, but I wonder if we can force the node positioning a bit to better express the important flow?

This kind of thing, but also with drivers and sensors leading into the estimator. screenshot from 2017-01-18 23-25-00

stephanbro commented 7 years ago

I think that it still looks a bit spaghetti-like, but it's thinned out enough that I think it could be understood without too much effort. I've updated the PR with the things we've talked about in this issue so we can start looking at the other builds and types if you want to add it to the CI like you suggested.

I like the idea you have of changing the node positions to have a better flow. Graphviz seems like it has a fair bit of customization as well as the ability to do sub-graphs that could probably organize things like the plot above. I will try to look in to it more to see what can be done.

bartslinger commented 7 years ago

Thanks for the great work so far! I would also love to see what it looks like if only the wakeup sources are shown, or perhaps in a different color.

bkueng commented 6 years ago

I took a shot at this as well. It's not easy to get something useful though, we have too many connections already! :)

Graphviz

The following uses graphviz, but with different placement algorithms (force-based to minimize the edge lengths) than above. This applies to the graphs:

These are some graphs that include all modules:

All modules from ROMFS: graph.fv.pdf

Only the SITL modules: graph.fv.pdf

D3.js

I then experimented with the D3.js library:

It's a really cool library. Try to hover over a node! For example when hovering over the sensor_accel topic, you can see all of our supported accel drivers.

What do you think?

@hamishwillee

dagar commented 6 years ago

Awesome!

If we could do something like selectable filters on the side (modules and topics), and filter presets for a few common configurations I'd find it immediately useful.

Then for different parts of the documentation we could highlight only the relevant modules and data flow.

LorenzMeier commented 6 years ago

@bkueng Same feedback: Awesome, if we have filters it would be easier to use.

hamishwillee commented 6 years ago

What he said https://github.com/PX4/Firmware/issues/6189#issuecomment-346890361

It is very cool, but it there is a lot going on. Filtering would help a lot.

What are a few questions you might ask that this system would help you answer? Ie imagine some test cases that would validate the final diagram. Then we will know it is "good enough". To get you started: "What uOrb topics have no subscribers (allows you to remove unused uOrb topics and reduce flash usage). So this would pass if we can easily highlight all unused topics.

bkueng commented 6 years ago

Thanks for your feedback and ideas. I agree the full graph is a bit too much. Although it can still be useful for certain cases (does not need to be the default view though).

I did the following updates:

This is the updated graph for FMUv4, it's already reduced quite a bit:

What do you think? Does it need to be reduced further?

@hamishwillee some use-cases that I see:

Next I'll look into auto-generating from certain cmake config targets (I've done that manually for v4 now) with a user-selectable list.