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

Remove actuators_pprz #3203

Draft
wants to merge 4 commits into
base: master
Choose a base branch
from
Draft

Remove actuators_pprz #3203

wants to merge 4 commits into from

Conversation

dewagter
Copy link
Member

@dewagter dewagter commented Dec 13, 2023

  1. there are now 3 types of numbers of actuators and 2 types of control command arrays:

    • SERVO_NAME_IDX -> actuators[] -> must also be used in ABI messages
    • INDI_MATRIX_SEQUENCE -> actuators_pprz[] is called actuator but contains controller outputs which not always map 1 to 1 to actuators
    • SERVO_NAME no="" -> CAN: should never be used in code. I would like to remove the define from the airframe.h file to avoid errors
    • the sequence of commands defined in the airframe file is often used but not always.
  2. there are now problems with adding virtual actuators when the number of commands becomes larger than the number of actuators and there are no checks if INDI_NUM_ACT is smaller than the actuator array size: see Rot mech fix pr #3180

  3. actuators_pprz index numbers are often the same numbers as the actuators, so many users confuse them, but they are not always the same. One is the sequence in the INDI and the other is the list of servos.

  4. SERVO_NAME_IDX and SERVO_NAME are sometimes confused in code: Hide actuator driver number from code #3206

  5. Safety features on commands are OVERRULED (e.g. FAILSAFE) when using INDI: command-laws in xml must re-implement functions like kill

  6. Command laws contain raw INDI indexes: not very readable + eff_sheduling relies on a specific order but cannot check this.

  7. RC_COMMANDS and RC_AUTO_COMMANDS are broken with INDI

  8. The simulator needs harmonized actuator commands (-MAX_PPRZ -> MAX_PPRZ), which required the actuators_pprz but this can be solved differently: actuators[ ] array of structs with pprz units and driver units #3205

  9. some modules write PWM directly to actuators and ignore the servo MIN and MAX

  • rotrocraft_cam
  • switch_servo
  1. there are unused commands in many airframe files: #Remove command_pitch and command_roll and command_yaw #3208

-> so I want to remove actuators_pprz: and instead do what was always the case before

control -> stabilization_cmd with the command sequence of the airframe file, and then set the actuators in the ActuatorsFromCommands block, just like the name says.

You can then:

  • make as many (virtual) commands as you want by adding commands
  • use default kill/failsafe functions
  • read the order of commands in the airframe instead of the code
  • forced to define which command goes where so no confusion with different numbering in actuator and actuator_pprz
  • no double numbering in actuators and actuators_pprz
  • RC_COMMANDS etc repaired...

@dewagter dewagter marked this pull request as draft December 14, 2023 10:11
@gautierhattenberger
Copy link
Member

@dewagter I'm all in favor of removing actuators_pprz !

  • In the "reference" design, the output of stabilization always goes to the commands vector, which is mapped to actuators with the command_law section of the airframe (and this is what you did). I also would like to be able to compile without the ROLL, PITCH, YAW command when they are not used. THRUST command is annoying because I thing it is used for the GCS display, but we could probably improve that as well.
  • From what I can see, you are now feeding the stabilization_cmd array. I will be needed to copy it to the real commands array at some point, and set_rotorcraft_commands
  • I'm not sure about the proper way to deal with the motors_on directly implemented in the airframe. It is really annoying and incompatible with dual MCU support (FBW doesn't have access to this variable).
  • Last point, I would like to change the actuators array of pprz_t by an array of structures (with the type of actuators, feedback value if available, current value in pprz and actuators units, etc). This can be done later I think, but could also help to manage the different status (kill, failsafe, motor on/off, running, etc).

@dewagter
Copy link
Member Author

dewagter commented Dec 14, 2023

Thanks a lot for the reply. I like the idea of actuators as an array of structs. In the meantime, I proposed an intermediate step to standardize it: #3205 but I like making structs even better.

SetRotorcraftCommands still needs to be done indeed. One option would be: https://github.com/paparazzi/paparazzi/pull/3203/files#diff-02e75cc9d71b8fb6997f5286311725c6d7cfc421e65da9e9b96f5e0a99ed46b2R133

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants