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

PX4 Actuator & Geometry Configuration and Testing UI #9952

Merged
merged 6 commits into from Dec 14, 2021
Merged

Conversation

bkueng
Copy link
Member

@bkueng bkueng commented Oct 27, 2021

This brings a new configuration page that goes together with the PX4 control allocation work (replacing the mixer config files with a parameter-based implementation).

It's using metadata from the fmu to dynamically create a UI matching the vehicle's hardware capabilities and geometry/airframe configuration.
For example this is how it looks like for a multirotor:
qgc_actuators_mc_aux

Or for a VTOL (tiltrotor) w/o configurable geometry:
qgc_actuators_vtol_tiltrotor

Video going through the UI: https://youtu.be/oKru4EsF1aM

Notes:

  • The geometry image is dynamically rendered based on the actuator position parameters, and only shown for multirotors. It's however possible to extend for different vehicle types later on
  • Tested with light & dark themes
  • A param change for any of the used params in the UI leads to a complete UI update. It's fast enough but could be made more fine granular.
  • I don't really like the setActuatorsController() in Vehicle, maybe @DonLakeFlyer you have a better approach?
  • The UI can be improved further, in particular I don't like the actuator actions buttons too much.

Some MC geometries:
test_geometry_2_3
test_geometry_2_4
test_geometry_2_5

TODO:

  • COMPONENT_INFORMATION API integration
  • Mavlink messages
  • metadata generation on the PX4 side
  • potentially add an example json to the mock link

@dagar
Copy link
Member

dagar commented Oct 27, 2021

How do you handle any differences in Z? Editing those parameters directly?

@bkueng
Copy link
Member Author

bkueng commented Oct 27, 2021

How do you handle any differences in Z? Editing those parameters directly?

There's an 'advanced' checkbox which reveals more columns. We can flag any column we'd like as advanced through the metadata.

@DonLakeFlyer
Copy link
Contributor

DonLakeFlyer commented Oct 28, 2021

I don't quite get the usage pattern for ActuatorsController. The pattern for a controller is that it is a standalone subclass of FactPanelController which is instantiated in the qml itself. This give qml access to parameters as well as any other C++ code it needs and it ties the lifetime of the controller to the UI element as well.

@bkueng
Copy link
Member Author

bkueng commented Oct 29, 2021

I don't quite get the usage pattern for ActuatorsController. The pattern for a controller is that it is a standalone subclass of FactPanelController which is instantiated in the qml itself. This give qml access to parameters as well as any other C++ code it needs and it ties the lifetime of the controller to the UI element as well.

Ok my understanding is more general then: it's just the business logic behind the QML. Lifetime is that of the UI, but I need to access it to check if the UI should be shown in the first place (https://github.com/mavlink/qgroundcontrol/pull/9952/files#diff-2b5b97c093e193f171f21aa413fb76025f9e505b45e4cf165643dad6eb0a5355R91), and then also for the setupComplete check (https://github.com/mavlink/qgroundcontrol/pull/9952/files#diff-195a638016c59606515d8d8f22c395ecef78d87c43af305b7d1c106e22441f72R55).

@DonLakeFlyer
Copy link
Contributor

Ok, I get it. Probably easier to discuss on a call though. Why don't you ping me on slack and we can talk.

@bkueng bkueng force-pushed the px4_actuators branch 2 times, most recently from 513fcb7 to fdd141e Compare November 12, 2021 06:32
@bkueng bkueng changed the title [WIP] PX4 Actuator & Geometry Configuration and Testing UI PX4 Actuator & Geometry Configuration and Testing UI Nov 12, 2021
@ryanjAA
Copy link

ryanjAA commented Dec 1, 2021

This is pretty awesome. Nice work! 👍

Particularly useful for servos, so it's clear which ones are controlled.
@TorbjornHouge
Copy link

This looks really good, great work!
If I understand it correctly, this eliminates the need for mixer file type configuration, but for fixed wing or VTOL style systems, I am not understanding how I with this system will be able to adjust my servo midpoints and travel. I see I can set min/max and failsafe, but setting different aileron deflection up/down (which today has to be done with Mixer) is not shown in the images above. Also, I suppose this change requires both an update to QGC and autopilot firmware to be functional.

@bkueng
Copy link
Member Author

bkueng commented Dec 10, 2021

Thanks! Midpoint (trim) is something I'm just adding (PX4/PX4-Autopilot#18776). The mechanism is flexible so that we can add/remove parameters on the autopilot side (also later on), without having to change QGC.

@dagar dagar merged commit c05428b into master Dec 14, 2021
@dagar dagar deleted the px4_actuators branch December 14, 2021 16:17
@ryanjAA
Copy link

ryanjAA commented Dec 14, 2021

I think there should be a hide option in qgc for this though. I can imagine the average user messing things up in a hurry if they edit this by accident.

This is awesome though. Excited to test this shortly.

@DonLakeFlyer
Copy link
Contributor

I think there should be a hide option in qgc for this though. I can imagine the average user messing things up in a hurry if they edit this by accident.

Which comes back to an age-old discussion around QGC "vehicle flyers" and QGC "vehicle developers". Right now QGC has the same UI for both which isn't really correct. QGC does have the concept of advanced mode but it is only used in custom builds. Right now regular QGC is always running in advanced mode.

@ryanjAA
Copy link

ryanjAA commented Dec 14, 2021

Which makes sense. I like the click 7 times option but I simply wonder if in the open this lets you really mess your vehicle up if you shouldn’t be in there. The platform/airframe change warning blocks or at least tells them (or should) when you are changing an airframe and restart but this is crucial on the development side, just scary on a user side. I don’t want to diminish the hard work by @bkueng this is awesome and so needed. Just thinking about people doing what people do…

I suppose the argument could be made that parameters can get changed and mess up your vehicle just as easily. I just was thinking of a hide option is the misc area in qgc but overall you are correct from advanced/dev to use mode.

@DonLakeFlyer has there ever been any talk to simply activating the Easter egg so people have the option in standard builds to block themselves out during normal use?

@DonLakeFlyer
Copy link
Contributor

has there ever been any talk to simply activating the Easter egg so people have the option in standard builds to block themselves out during normal use?

Yes. But it requires some work to make it work well in regular QGC. It's not as much of an easter than than a setting in that case. Also I don't thinkg things are necessarily marked as advanced correctly in the regular build. Custom builds tends to do that on their own.

@TorbjornHouge
Copy link

Maybe one solution is not to use static "developer" and "pilot" versions or modes, but using setupfiles that can be tailored to the individual (or company, or product) preference. I have used systems allowing for that more than a decade, and find it works great, also when it comes to transferring personal settings after upgrades. Different user groups and companies have different priorities and preferences. Some like to see much information and be able to act on it, some (me flying BLOS as an example) prefer hiding what I cannot utilize at the time. When it comes to what settings available to which users, it is hard to know what is need to have, nice to have, too much, etc. It may vary between applications, so an option to show/hide based on config file may be a possible suggestion for that as well. I really like the thought of enabling flexibility and configurability, and if possible, allowing the end user to have a say. That said, my use is flying fixed wings, from what I have used PX4 for multirotors I can see the argument for more static "developer" and "flyer" versions (I realize I might be in the advanced category for fixed wing, and basic user of multirotors).

@ryanjAA
Copy link

ryanjAA commented Jan 28, 2022

Been looking at this. It's pretty awesome.

What's needed for it to handle a pusher motor (for a vtol) as well as setups that have each output channel to only one servo in fw? For instance I see "RC Roll" but how can we break that out for servo 1 and servo 2 (left and right aileron, same with flaps, Vtail ruddervators) to their own output? That seems like it might be as simple as assigning toll to output 1 and output 2. Trimming comes to mind for those scenarios

The other thing is mixing for V tail and similar, is there a way to do that in this?

This is great!

Edit: It's all in the CA_params - Awesome!

junwoo091400 added a commit to junwoo091400/qgroundcontrol that referenced this pull request Nov 1, 2022
Description
This is an initial porting (mash-up) of the files from standalone Frames visualization tool:
junwoo091400/PX4-Airframe-Metadata-support@75d54c4
in the structural form of the Actuators metadata QGC PR: mavlink#9952

Files mapped:
<Original Qt UI> -> <QGC (here)>

framecomponent -> src/Vehicle/CompInfoFrames
frames -> src/Vehicle/Frames/Frame
(ActuatorComponent from Actuators metadata PR) ->
src/AutoPilotPlugins/Px4/FrameComponent

main.qml -> src/AutoPilotPlugins/PX4/FrameComponentMain.qml
Frame.qml -> src/AutoPilotPlugins/Px4/Frame.qml

Note:
In the original standalone Qt application, I used the framecomponent as
the sole source for the main QML file (main.qml), however discovered
that in QGC we abstract away all the details of parsing the JSON and
such inside the `src/Vehicle`, and keep the `FrameComponent` files
clean, as it gets derived from the `VehicleComponent` class already (QGC
Custom).

Therefore, the `FrameComponent` files were dirty-copied from the
Actuators Metadata PR, as I didn't have a structure for it. And the
`framecomponent` files were renamed into `CompInfoFrames` file, as it
will be handling the parsing of JSON and such.

Otherwise, there were some Class renaming / useless singleton removal
(from `framecomponent` original source), and other changes, but these
are pretty straight forward.

This commit builds in QGC. But doesn't provide functional changes yet.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants