Support custom per-channel parameters in actuator driver

[SalimTerryLi]

Describe problem solved by this pull request

I'm now doing the rewrite of pca9685_pwm_out driver and want to make it able to output duty cycle information(aka PWM mode) instead of typical 1ms~2ms pulse signal.

This PR will let actuator drivers be able to do runtime configuration of its output, based on Dynamic Allocator in a more flexible way than before.

Describe your solution

Both param and meta generator are affected.

Test data / coverage

This is the part of my current module.yml:

  output_groups:
    - param_prefix: PCA9685
      group_label: 'PCA9685'
      channel_label: 'Channel'
      standard_params:
        disarmed: { min: 0, max: 4095, default: 900 }
        min: { min: 0, max: 4095, default: 1000 }
        max: { min: 0, max: 4095, default: 2000 }
        failsafe: { min: 0, max: 4095 }
      custom_params:
        pwm:
          label: PWM Mode
          prefix: PWM
          description: |
            Enable PWM mode will make PCA9685 output duty cycle instead of PPM pulse.
            Using MIN value different from 0 will lead to output (MIN/4096)% instead of 0%.
            Same applies to MAX and other fields.

          advanced: true
          param:
            type: boolean
            default: false
      num_channels: 16

After touching some of params by

pxh> param touch PCA9685_PWM1
pxh> param touch PCA9685_PWM2
pxh> param touch PCA9685_PWM3

Here are some screenshots of the working demo:

[SalimTerryLi]

Bitfield param is implemented in a non-per-channel basis. Have no idea how to support that now

[junwoo091400]

@SalimTerryLi Thanks for this PR! Could you apply the changes requested?

[SalimTerryLi]

@SalimTerryLi Thanks for this PR! Could you apply the changes requested?

Sorry for the inconvenience. Will do it next month.

[SalimTerryLi]

Hi @bkueng ,

While playing around with schema validation, I come up with some issues:

• Since the actuator params requires some display settings from actuators.schema.json like show_as, is it OK to duplicate them in module_schema.yaml? Or any way to reference those defined in json from yaml?
• Currently I directly pulled the general *parameter_definition into custom_params, there seems to be some unappropriated fields like num_instances. Should I:
  • Override those definition with void and patch those things inside generate_params.py
  • Leave them as-is
  • Move all of custom parameters into module level, just as you already suggested. But that may cause trouble in generate_actuators_metadata.py for unlinking params from actuator context?
    • Always pull in all params under PWM group is acceptable?
    • Mark those custom params with some fields and check for it?

My current module.yaml:

module_name: PCA9685 Output
actuator_output:
  output_groups:
    - param_prefix: PCA9685
      channel_label: 'Channel'
      standard_params:
        disarmed: { min: 800, max: 2200, default: 900 }
        min: { min: 800, max: 1400, default: 1000 }
        max: { min: 1600, max: 2200, default: 2000 }
        failsafe: { min: 800, max: 2200 }
      custom_params:
        MODE_${i}:
          description:
            short: Set output signal mode on ${i}
            long: |
              FOO BAR
          type: enum
          values:
            0: PWM
            1: Duty-cycle
          default: 0
          num_instances: 16
          instance_start: 1
          show-as: 'bitmap'
      num_channels: 16

[bkueng]

Since the actuator params requires some display settings from actuators.schema.json like show_as, is it OK to duplicate them in module_schema.yaml? Or any way to reference those defined in json from yaml?

Yes certainly. Whatever is in the json, we can use in the yaml.

Move all of custom parameters into module level, just as you already suggested. But that may cause trouble in generate_actuators_metadata.py for unlinking params from actuator context?

I would go with that, but I'm not sure what problems you might run into. Can you elaborate?

[SalimTerryLi]

@bkueng Sorry but I'm still confused by some of your statement... I'd be appreciate if further guidance is possible...

And extend the schema: https://github.com/PX4/PX4-Autopilot/blob/main/validation/module_schema.yaml#L285

This goes into actuator_output -> output_groups, which is I currently have done: by adding new custom-params subgroup. But that means the parameters are not located at module level

With that, bitfield params are supported as well: you would declare a single parameter, and then in the output section use

Which output section here is referenced against?

I would go with that, but I'm not sure what problems you might run into. Can you elaborate?

If I re-arrange the definition into module level, then how could I tell if one param is needed to be included by generate_actuators_metadata.py? Because if the param is located inside output_groups then it must be placed under custom-params subgroup, so that it is much easier to process those params into GUI. But if the param is located like:

module_name: PCA9685 Output
actuator_output:
  output_groups:
    - param_prefix: PCA9685
      channel_label: 'Channel'
      standard_params:
        disarmed: { min: 800, max: 2200, default: 900 }
        min: { min: 800, max: 1400, default: 1000 }
        max: { min: 1600, max: 2200, default: 2000 }
        failsafe: { min: 800, max: 2200 }
      num_channels: 16

parameters:
  - group: Actuator Outputs
    definitions:
      MODE_${i}:
        description:
          short: Set output signal mode on ${i}
          long: |
            FOO BAR
        type: enum
        values:
          0: PWM
          1: Duty-cycle
        default: 0
        num_instances: 16
        instance_start: 1
        show-as: 'bitmap'

then There is no existing way to tell whether MODE_${i} should be processed by generate_actuators_metadata.py or not?

Also this raises another question: where should I extend the scheme? &parameter_definition seems not be a good place for that...

I'm trying to address # TODO: support non-standard per-channel parameters but still can not find to the right path...

[bkueng]

Sorry I wasn't clear enough then. My idea was to do it like this:

module_name: PCA9685 Output
actuator_output:
  output_groups:
    - param_prefix: PCA9685
      channel_label: 'Channel'
      standard_params:
        disarmed: { min: 800, max: 2200, default: 900 }
        min: { min: 800, max: 1400, default: 1000 }
        max: { min: 1600, max: 2200, default: 2000 }
        failsafe: { min: 800, max: 2200 }
      custom_params:
        - name: 'PCA9685_MODE_${i}'
          label: 'Enable PWM Mode'
          # show_as: bitset # if a single parameter is used
      num_channels: 16

parameters:
  - group: Actuator Outputs
    definitions:
      MODE_${i}:
        description:
          short: Set output signal mode on ${i}
          long: |
            FOO BAR
        type: enum
        values:
          0: PWM
          1: Duty-cycle
        default: 0
        num_instances: 16
        instance_start: 1

[SalimTerryLi]

Sincerely thanks @bkueng

So far so good, everything works as intended. My current module.yml:

module_name: PCA9685 Output
actuator_output:
  output_groups:
    - param_prefix: PCA9685
      channel_label: 'Channel'
      standard_params:
        disarmed: { min: 0, max: 4095, default: 900 }
        min: { min: 0, max: 4095, default: 1000 }
        max: { min: 0, max: 4095, default: 2000 }
        failsafe: { min: 0, max: 4095 }
      custom_params:
        - name: 'DUTY_EN'
          label: "Duty-Cycle\n Mode"
          index_offset: -1
          show_as: bitset
      num_channels: 16

parameters:
  - group: Actuator Outputs
    definitions:
      PCA9685_DUTY_EN:
        description:
          short: Set output signal mode on ${i}
          long: |
            FOO BAR
        type: bitmask
        bit:
          0: Put CH1 to Duty-Cycle mode
          1: Put CH2 to Duty-Cycle mode
          2: Put CH3 to Duty-Cycle mode
          3: Put CH4 to Duty-Cycle mode
          4: Put CH5 to Duty-Cycle mode
          5: Put CH6 to Duty-Cycle mode
          6: Put CH7 to Duty-Cycle mode
          7: Put CH8 to Duty-Cycle mode
          8: Put CH9 to Duty-Cycle mode
          9: Put CH10 to Duty-Cycle mode
          10: Put CH11 to Duty-Cycle mode
          11: Put CH12 to Duty-Cycle mode
          12: Put CH13 to Duty-Cycle mode
          13: Put CH14 to Duty-Cycle mode
          14: Put CH15 to Duty-Cycle mode
          15: Put CH16 to Duty-Cycle mode
        default: 0

Still a small issue, as I commented in code:

check and match the custom params in output_groups with module-level parameters

Since those two part of param is totally independent, there's no way to validate and ensure the one in actuator_metadate is actually existing as module-level param. Any thought?

[bkueng]

Looks good, is it ready to merge?

Since those two part of param is totally independent, there's no way to validate and ensure the one in actuator_metadate is actually existing as module-level param. Any thought?

Right, I don't think it's a big problem in practice, as a mismatch will show up immediately when testing.
But if you want to add a check that would certainly not be bad.

[SalimTerryLi]

Rebased. I'd leave the additional check as a TODO for now :) Lots' of things need to be done before...

@bkueng