I haven't tested yet, but from a quick look at the code it seems to be primarily for the MAVLink inspector. Would it be possible/difficult to also add for the MAVLink Actions definitions?

That can be left as a later PR if need be (dedicated to addressing #2286) - just not sure how complex it would be.

I haven't tested yet, but from a quick look at the code it seems to be primarily for the MAVLink inspector. Would it be possible/difficult to also add for the MAVLink Actions definitions?

That can be left as a later PR if need be (dedicated to addressing #2286) - just not sure how complex it would be.

I just want to get the metadata in, after that others can add the integration in other components in cockpit, the minimal integration that this PRs delivers is a proof of concept.

Nice addition!

Just one thing to fix: right now long descriptions are taking the whole screen width. Can we modify the tooltip to instead break lines with a max width?

limited to 128 chars

It seems to have a pretty significant performance impact, especially when opening the inspector (which also seems to use extra memory every time it opens), as well as adding a bunch of variance to the app framerate - especially when tracking a variable's state:

Was this performance problem there already or is it something new?

Was this performance problem there already or is it something new?

I confirmed it's not in master before posting it. I meant to include "on master it's not even noticeable when you open the pane or track a variable." in my original review comment.

I'm unsure how to apply this, but I believe it should dynamically load the tooltip text on hover, instead of loading the potential tooltips for every possible message when the window opens. If need be we could batch that with pagination for the message list, at the expense of the current nice scrolling experience, but I think it should be fast enough to just fetch them individually as and when needed.

I also think it might be currently tying the tooltip too deeply to the tracked messages, which could be causing it to reload something it shouldn't be when new messages are triggered as received/sent, and update the interface with the new values.

That said, these are frontend implementation issues (technically unrelated to the inclusion of the MAVLink metadata), so if @patrickelectric doesn't want to fix them then this PR could be left as a draft and used as a reference for one that pulls things in without the performance issues :-)

I can confirm the same problem in master, but not that bad. The FPS drops from 60 -> 30 when opening a message. Will investigate.

That said, these are frontend implementation issues (technically unrelated to the inclusion of the MAVLink metadata), so if @patrickelectric doesn't want to fix them then this PR could be left as a draft and used as a reference for one that pulls things in without the performance issues :-)

Or we could merge only the two first commits, that add the metadata and the interface to access them.

@patrickelectric if you still want to add the metadata somewhere and this fix ends up getting too much time, a suggestion would be to add the tips to the MavlinkMessageActionConfig.vue when a user selects a message from the dropdown. I think it has even more value there.

Thanks @rafaellehmkuhl but I have problems with performance issues 🤣
I did found out the issue, will add another commit soon. Now it's in solid 60 fps.

@ES-Alexander @rafaellehmkuhl the PR is better and faster now

Waiting for @rafaellehmkuhl and/or @ArturoManzoli to merge

@rafaellehmkuhl the tooltips are truncated now, so you can presumably dismiss your requested change review.

That said, I agree with you that in principle the text should be wrapped instead, so we should probably raise an Issue for it.

I did briefly look into how to do tooltip wrapping and couldn't find a reasonable solution (there was lots of search pollution from other Vue variants, that have custom tooltips), so I'm unclear whether it's a trivial or unexpectedly complex task 🤷‍♂️

@ES-Alexander @patrickelectric the wrapping thing is definitely not a blocker. It can be worked out later.

Can you just confirm that the points around performance degradation that Eliot raised on his previous comment were fixed? If so we can definitely merge it.

Can you just confirm that the points around performance degradation that Eliot raised on his previous comment were fixed?

1. This PR does not introduce any new performance issues
2. It does make some existing ones slightly more significant, likely just from loading some extra data to throw around
   • I still see a technically noticeable lag spike when the MAVLink messages page is opened, but it's mostly noticeable on the framerate graph - it only hangs the interface for a fraction of a second, during which the user is expecting a menu to be opening anyway
   • This (and the repeatedly increasing but technically capped memory I mentioned) seems to be related to how JS loads things, since it's a problem with the data lake page too - it's just not super noticeable on my system unless it's under load (like during a video call with screen sharing 😅)
     • It's possible this is fixable, but might also just be a technology limitation of JS's memory management, and it's not clear how many places the issue is present in
3. There was one existing performance issue that this PR was making substantially more significant (i.e. the reloading of the entire pane on every JSON update), but the PR has been refactored to fix that issue (only the relevant bit reloads now), and I've confirmed it restores performance to a reasonable level (at least on my machine, and Patrick's) even with the new additions 👍