Introduce actuator output function parameters #18265

bkueng · 2021-09-20T07:57:01Z

Unifies the control allocation builds & adds a param SYS_CTRL_ALLOC
Introduces output functions as described in https://docs.google.com/document/d/10WsNyjARZEuDeCNywnwBnUy3FhX4CR2fmSL0b4eZl6U/edit?pli=1#heading=h.8g8ygrtolpw8
- Only if SYS_CTRL_ALLOC is set, the output params are used, otherwise existing behavior is used
- Remove direct mixer for control allocation
- Add servos to uavcan as an example
- The structure to handle output functions is generic, making it easy to add new functions and subscribing to any topic (e.g. 4b59357)
fmu pwm outputs:
- parse timer_config.cpp and generate per-timer config params
- add capture pins to pwm pins for boards that have them (with matching param label):
  - fmu-v5: 3 additional pins, 11 total now
  - durandal-v1: 5 additional pins, 10 total now
  - pix32v5: 3 additional pins, 11 total now
  - fmu-v5x, fmu-v6u, fmu-v6x: 1 additional pin, 9 total now
output param naming: PWM_FMU_* is used for FMU, SITL and HIL_ACT_* for HITL outputs. I'm not completely set on these though.

This can go in before or after the IO mixing change (#16444).

See individual commits for details.

dagar · 2021-09-20T14:25:32Z

output param naming: PWM_FMU_* is used for FMU, SITL and HIL_ACT_* for HITL outputs. I'm not completely set on these though.

This is borderline bike-shedding, but should we try to settle this one now? (previous discussion https://docs.google.com/document/d/10WsNyjARZEuDeCNywnwBnUy3FhX4CR2fmSL0b4eZl6U/edit?pli=1&disco=AAAANoXLsW0)

There are kind of 2 parts of this that I question (mildly).

Perpetuating legacy naming everywhere that we could instead be close to deprecating. I'm in favor of these outputs being named as close as possible to what's user facing. I believe the majority of boards have been and continue to be MAIN/AUX on the case with only a few exceptions. Even on px4_fmu-v5 the pixhawk 4 says has ports labelled FMU and I/O, but on the pixhawk 4 mini it's MAIN, and on the CUAV board v5 it's MAIN/AUX.
The idea that we're going to get users to do the right thing because default airframes will now go to FMU (AUX) first. I think this is going to seem pretty odd for all but the most technical users that understand the full picture and history. The availability of DSHOT on only some outputs and trivial airframe output/mixing configuration in the GUI will take care of this for the majority of the cases where it might even matter.

I actually don't think we should even be carrying opinionated default airframes that we pretend will work everywhere. There are a handful of cases where exact configurations should be carried. For the rest it should be a generic start. A "generic quadcopter" should default to using 4 MAIN outputs, but otherwise carry no actuator specifics. Then on the GUI side it should be easy for a user to switch those over to DSHOT, UAVCAN, etc on arbitrary outputs and set any specifics.

dagar · 2021-09-20T14:33:46Z

Is "Actuator Set X" for Mavlink MAV_CMD_DO_SET_SERVO?

Even with the potential for conflicts what if we just let people use servo directly? We could do something like reject the command if Servo X is in use by control allocation, etc.

bkueng · 2021-09-21T06:33:07Z

This is borderline bike-shedding, but should we try to settle this one now?

We might as well.

I'm in favor of these outputs being named as close as possible to what's user facing.

Agreed. I don't think it will matter much though in this case, as the config UI will not even show the param name, just the label.

I think this is going to seem pretty odd for all but the most technical users that understand the full picture and history

Odd yes, but is it really a problem? They can ask and there's a good reason. It is a historical problem that we have to carry.
Most people will probably not know about any downsides and just stick to the default.

Overall I tend towards PWM_FMU_, but really only slightly.

I actually don't think we should even be carrying opinionated default airframes that we pretend will work everywhere. There are a handful of cases where exact configurations should be carried. For the rest it should be a generic start. A "generic quadcopter" should default to using 4 MAIN outputs, but otherwise carry no actuator specifics. Then on the GUI side it should be easy for a user to switch those over to DSHOT, UAVCAN, etc on arbitrary outputs and set any specifics.

I see that the same way.

Is "Actuator Set X" for Mavlink MAV_CMD_DO_SET_SERVO?

No, it's for DO_SET_ACTUATOR. I did not plan to add support for MAV_CMD_DO_SET_SERVO, but if needed I'd rather map it to the (future) actuator test interface.

jlecoeur · 2021-09-21T07:27:26Z

Is "Actuator Set X" for Mavlink MAV_CMD_DO_SET_SERVO?

No, it's for DO_SET_ACTUATOR. I did not plan to add support for MAV_CMD_DO_SET_SERVO, but if needed I'd rather map it to the (future) actuator test interface.

What about "MAVLink X" or "MAVLink Actuator X" or "MAVLink Set X", or again "MAVLink Input X".
This would directly hint at the link with MAVLink (I first thought it was a group "set" of actuators).

dagar · 2021-09-21T13:33:32Z

Is "Actuator Set X" for Mavlink MAV_CMD_DO_SET_SERVO?

No, it's for DO_SET_ACTUATOR. I did not plan to add support for MAV_CMD_DO_SET_SERVO, but if needed I'd rather map it to the (future) actuator test interface.

What about "MAVLink X" or "MAVLink Actuator X" or "MAVLink Set X", or again "MAVLink Input X".
This would directly hint at the link with MAVLink (I first thought it was a group "set" of actuators).

If it makes sense I'm hoping we can skip including "Mavlink" in the naming, then continue using the vehicle_command in a ROS2 world.

dagar · 2021-09-21T13:37:50Z

src/drivers/pwm_out/module.yaml

+        type: enum
+        default: 400
+        values:
+            -5: DShot150


What about boards that don't have dshot (kinetis, imxrt, etc)?

They're automatically removed. Also for timers w/o DMA.

dagar · 2021-09-21T13:56:20Z

src/drivers/pwm_out/module.yaml

+            -5: DShot150
+            -4: DShot300
+            -3: DShot600
+            -2: DShot1200


So this means using PWM_FMU_* parameters to configure dshot min/max/disarm/etc as well?

Can't we have DSHOT_* parameters per physical output configured to dshot? Then we'd have appropriate defaults for min, disarmed, etc and the PWM parameters could have units in microseconds.

No, dshot uses separate params. Just the function is the same.

dagar · 2021-09-21T15:23:35Z

I'm in favor of these outputs being named as close as possible to what's user facing.

Agreed. I don't think it will matter much though in this case, as the config UI will not even show the param name, just the label.

Maybe this is where we disagree. I don't think we need to have much of a disconnect between the param name and the label. We should map these parameters to particular physical outputs unambiguously on a per board basis.

For example on the original pixhawk (and pixhawk 2, etc) we have 8 MAIN and 6 AUX outputs clearly labelled. PWM_MAIN_* can always refer to those px4io outputs regardless of it being active or not (SYS_USE_IO).

In the fmu-v5 case it would be set early in boot based on hardware version, but then the location of PWM_MAIN and PWM_AUX is effectively static per board.

I think this is going to seem pretty odd for all but the most technical users that understand the full picture and history

Odd yes, but is it really a problem? They can ask and there's a good reason. It is a historical problem that we have to carry.
Most people will probably not know about any downsides and just stick to the default.

The transition is potentially problematic depending on how it's handled for the remaining in tree airframes.

Other than that's it's going to look like a PX4 oddity that will have to be explained again and again. From historical usage many people will have certain expectations for MAIN and AUX. I don't think it's worth fighting that existing experience, but we can still move away from it going forward with labeling on newer boards and an increasing number of boards dropping the px4io entirely.

I don't want to try and trick people into doing something slightly better in the face of 10 years of convention.

jlecoeur · 2021-09-21T15:34:38Z

Is "Actuator Set X" for Mavlink MAV_CMD_DO_SET_SERVO?

No, it's for DO_SET_ACTUATOR. I did not plan to add support for MAV_CMD_DO_SET_SERVO, but if needed I'd rather map it to the (future) actuator test interface.

What about "MAVLink X" or "MAVLink Actuator X" or "MAVLink Set X", or again "MAVLink Input X".
This would directly hint at the link with MAVLink (I first thought it was a group "set" of actuators).

If it makes sense I'm hoping we can skip including "Mavlink" in the naming, then continue using the vehicle_command in a ROS2 world.

Right, then "Offboard X" might be a better fit

jlecoeur · 2021-09-21T15:50:32Z

src/modules/control_allocator/ControlAllocator.cpp

-	_actuator_controls_5_pub.publish(actuator_controls_5);
+	_actuator_motors_pub.publish(actuator_motors);
+
+	// TODO: servos


I am not sure to see the point of using separate topics.

We will have unused actuator data in both topics, which IMO hinders the debugging and logging experience

I do not buy the pre-arm argument. Any output can be unsafe to activate pre-flight, we should not infer it from its type. We should rather get it from an output-level "PREARMABLE" parameter, effective for control_alloc function and other functions.

Control allocator will need parameters to decide which output goes to the motor topic and which goes to the servo topic

I do not buy the pre-arm argument. Any output can be unsafe to activate pre-flight, we should not infer it from its type. We should rather get it from an output-level "PREARMABLE" parameter, effective for control_alloc function and other functions.

Having this safety property at the lowest level makes sense to me if ultimately we can have the feedback/status tightly integrated. The higher level component telling the landing gear to retract, or the gimbal to deploy, or the engine to start should be able to know (easily) that the command isn't possible at this stage of operation.

IMO we generally need a bit more semantics, be it for ease of configurability & use or system inspection. While at the same time it does not reduce generality.

Control allocator will need parameters to decide which output goes to the motor topic and which goes to the servo topic

It knows this already.

JacobCrabill · 2021-09-21T18:56:56Z

Unfortunately I'm not sure when I'll have the chance to dig through this in detail, but at a high level, what's different between this PR and #16808?

bkueng · 2021-09-22T06:11:38Z

In the fmu-v5 case it would be set early in boot based on hardware version, but then the location of PWM_MAIN and PWM_AUX is effectively static per board.

This is the case that doesn't work. You'd have to select the metadata as well dynamically. That's possible, but not something I want to add.

Alright, let's go with PWM_MAIN/AUX then.

Unfortunately I'm not sure when I'll have the chance to dig through this in detail, but at a high level, what's different between this PR and #16808?

The main one is that outputs can directly subscribe to any topic, not just actuator_controls.

Right, then "Offboard X" might be a better fit

That's fine with me, but you can also use it in missions.

bkueng · 2021-10-05T12:28:01Z

Updates:

changed params to PWM_MAIN_*
rebased & handling px4io
renamed Actuator_Set -> Offboard_Actuator_Set

dagar · 2021-10-06T19:52:21Z

I've resolved the remaining build failures. I actually disagree with the offboard part of Offboard_Actuator_Set, but overall this is such an important improvement we can settle these little details later.

Is everyone else happy to move forward with this?

dagar · 2021-10-07T13:24:54Z

Over on px4_fmu-v6x_default.

/opt/gcc/bin/../lib/gcc/arm-none-eabi/9.3.1/../../../../arm-none-eabi/bin/ld: px4_fmu-v6x_default.elf section `.text' will not fit in region `flash'
1783
/opt/gcc/bin/../lib/gcc/arm-none-eabi/9.3.1/../../../../arm-none-eabi/bin/ld: region `flash' overflowed by 15649 bytes
1784
collect2: error: ld returned 1 exit status

…targets

This happened with multi-instance, when the second instance did not load a mixer. Then the first instance would always return false in update_pwm_out_state, and continuously call up_pwm_servo_arm. This led to irregular pulses, e.g. with an output set to fixed 1500us, I saw pulses of e.g. 1800us occationally (they were in a range of [1500-2100]).

The parameter additions are only used internally for the pwm generator.

…tivenessMatrix clang tidy error: /__w/PX4-Autopilot/PX4-Autopilot/src/modules/control_allocator/ActuatorEffectiveness/ActuatorEffectivenessMultirotor.cpp:50:34: error: default arguments on virtual or override methods are prohibited [google-default-arguments,-warnings-as-errors] ActuatorEffectivenessMultirotor::getEffectivenessMatrix(matrix::Matrix<float, NUM_AXES, NUM_ACTUATORS> &matrix,

The implementation assumed timers are defined in the same order as used in the channels. This could lead to a mismatch between TIMx param and actual timer config. Now we use the actual array index, same as in the code.

So that the ordering of the generated params make more sense. TIM1 is now for channels 2-4 instead of 5-8.

The defaults changed in the previous commit for per-channel params, so we make sure that the overall params are still used as long as the per-channel params are default.

So that all failures can be evaluated.

…ance This fixes the case where oneshot was enabled with multi-instance pwm_out, triggering oneshot updates too close to each other and as a result could lead to spinning motors while disarmed.

bkueng · 2021-10-18T09:58:40Z

Rebased, CI passing. Should be good to go from my side if there's nothing else.

dookei · 2022-02-14T10:45:09Z

Hi, I am currently trying to use this PR. Unfortunately I am not able to find any reference to PWM_MAIN_FUNC exept on the mixer files. As this been tested? I would just like to control a few servos from an application.

DronecodeBot · 2023-08-15T13:08:37Z

This pull request has been mentioned on Discussion Forum for PX4, Pixhawk, QGroundControl, MAVSDK, MAVLink. There might be relevant details there:

https://discuss.px4.io/t/using-do-set-actuator-to-releasing-the-payload/28686/3

bkueng force-pushed the ctrl_alloc_function_params branch from 515f332 to 0ae7ede Compare September 20, 2021 08:42

dagar self-requested a review September 20, 2021 13:23

dagar added the Admin: Enhancement (improvement) 💡 label Sep 20, 2021

dagar reviewed Sep 21, 2021

View reviewed changes

jlecoeur reviewed Sep 21, 2021

View reviewed changes

mrpollo added the [ARCHIVED] dev call (Use "Maintainers call") label Sep 21, 2021

bkueng force-pushed the ctrl_alloc_function_params branch 2 times, most recently from 9cc8422 to 987f46b Compare October 5, 2021 11:40

dagar force-pushed the ctrl_alloc_function_params branch from f45611f to 142f4b3 Compare October 6, 2021 19:49

dagar previously approved these changes Oct 6, 2021

View reviewed changes

bkueng dismissed dagar’s stale review via 6b6e72f October 7, 2021 08:59

bkueng force-pushed the ctrl_alloc_function_params branch from 142f4b3 to 6b6e72f Compare October 7, 2021 08:59

bkueng force-pushed the ctrl_alloc_function_params branch 2 times, most recently from 92a86c6 to 5b1858b Compare October 8, 2021 11:26

bkueng added 3 commits October 18, 2021 10:32

commander: correct lockdown CLI description

b65e26e

fix mixer_module: use PRIx32 for debug printf

731f902

control allocation: introduce parameter SYS_CTRL_ALLOC & unify build …

abdfec0

…targets

bkueng and others added 17 commits October 18, 2021 10:34

module_schema.yaml: add actuator_output section

d69ac4d

The parameter additions are only used internally for the pwm generator.

output_groups_from_timer_config.py: fix timer index

d75ee15

The implementation assumed timers are defined in the same order as used in the channels. This could lead to a mismatch between TIMx param and actual timer config. Now we use the actual array index, same as in the code.

boards/px4/io-v2: swap timer definitions

63473ab

So that the ordering of the generated params make more sense. TIM1 is now for channels 2-4 instead of 5-8.

params: use PWM_MAIN_* instead of PWM_FMU_*

ae2fd5f

px4io, pwm_out: update pwm default checking

ef372d9

The defaults changed in the previous commit for per-channel params, so we make sure that the overall params are still used as long as the per-channel params are default.

px4io: add dynamic mixing support

18a4218

ROMFS: disable vtol airframes for kakutef7

bc9144a

nxp/fmurt1062-v1: set CONSTRAINED_FLASH

27b9beb

cubepilot/cubeorange test: disable uavcan to reduce flash usage

718d48b

ci compile_nuttx: disable fail-fast

010b5f2

So that all failures can be evaluated.

boards: px4_fmu-v3_test disable modules to save flash

a5ccc0f

boards: px4_fmu-v5_uavcanv0periph disable modules to save flash

4e5b8ef

boards: px4_fmu-v6x_default disable modules to save flash

e1be42a

fix pwm: only update oneshot timers owned by the current pwm_out inst…

e54288a

…ance This fixes the case where oneshot was enabled with multi-instance pwm_out, triggering oneshot updates too close to each other and as a result could lead to spinning motors while disarmed.

px4/fmu-v5 test, uavcanv0periph: disable modules to reduce flash

564e8a8

bkueng force-pushed the ctrl_alloc_function_params branch from 5b1858b to 564e8a8 Compare October 18, 2021 08:34

dagar approved these changes Oct 18, 2021

View reviewed changes

dagar merged commit 3e9c88e into master Oct 18, 2021

dagar deleted the ctrl_alloc_function_params branch October 18, 2021 22:45

JacobCrabill mentioned this pull request Oct 18, 2021

Configurable Actuator Output Control #16808

Closed

bkueng mentioned this pull request Oct 19, 2021

drivers/px4io cpu usage problem after running for long periods #18375

Closed

jlecoeur mentioned this pull request Oct 21, 2021

airframes: add standard_vtol_ctrlalloc model #18479

Closed

dookei mentioned this pull request Feb 14, 2022

actuator output function, how to use it ? #19190

Closed

junwoo091400 mentioned this pull request May 5, 2022

[Project Tracker] Control Allocation User experience improvement #19444

Open

19 tasks

junwoo091400 mentioned this pull request Aug 15, 2023

[BUG] Support for DO_SET_ACUTUATOR and DO_SET_SERVO MAVLink commands #21966

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Introduce actuator output function parameters #18265

Introduce actuator output function parameters #18265

bkueng commented Sep 20, 2021

dagar commented Sep 20, 2021

dagar commented Sep 20, 2021

bkueng commented Sep 21, 2021

jlecoeur commented Sep 21, 2021 •

edited

Loading

dagar commented Sep 21, 2021

dagar Sep 21, 2021

bkueng Sep 22, 2021

dagar Sep 21, 2021

bkueng Sep 22, 2021

dagar commented Sep 21, 2021

jlecoeur commented Sep 21, 2021

jlecoeur Sep 21, 2021

dagar Sep 21, 2021

bkueng Sep 22, 2021

JacobCrabill commented Sep 21, 2021

bkueng commented Sep 22, 2021

bkueng commented Oct 5, 2021

dagar commented Oct 6, 2021

dagar commented Oct 7, 2021

bkueng commented Oct 18, 2021

dookei commented Feb 14, 2022

DronecodeBot commented Aug 15, 2023

Introduce actuator output function parameters #18265

Introduce actuator output function parameters #18265

Conversation

bkueng commented Sep 20, 2021

dagar commented Sep 20, 2021

dagar commented Sep 20, 2021

bkueng commented Sep 21, 2021

jlecoeur commented Sep 21, 2021 • edited Loading

dagar commented Sep 21, 2021

dagar Sep 21, 2021

Choose a reason for hiding this comment

bkueng Sep 22, 2021

Choose a reason for hiding this comment

dagar Sep 21, 2021

Choose a reason for hiding this comment

bkueng Sep 22, 2021

Choose a reason for hiding this comment

dagar commented Sep 21, 2021

jlecoeur commented Sep 21, 2021

jlecoeur Sep 21, 2021

Choose a reason for hiding this comment

dagar Sep 21, 2021

Choose a reason for hiding this comment

bkueng Sep 22, 2021

Choose a reason for hiding this comment

JacobCrabill commented Sep 21, 2021

bkueng commented Sep 22, 2021

bkueng commented Oct 5, 2021

dagar commented Oct 6, 2021

dagar commented Oct 7, 2021

bkueng commented Oct 18, 2021

dookei commented Feb 14, 2022

DronecodeBot commented Aug 15, 2023

jlecoeur commented Sep 21, 2021 •

edited

Loading