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

Add param COM_PREARM_MODE #12806

Merged
merged 6 commits into from
Sep 2, 2019
Merged

Add param COM_PREARM_MODE #12806

merged 6 commits into from
Sep 2, 2019

Conversation

jlecoeur
Copy link
Contributor

@jlecoeur jlecoeur commented Aug 25, 2019

replacement of #12713

  • Add param COM_PREARM_MODE
    Condition to enter the prearmed state, an intermediate state between disarmed and armed
    in which non-throttling actuators are active.
    value 0 Disabled
    value 1 Safety button
    value 2 Always

  • IO: respect actuator_armed.prearmed flag
    Arm non-throttling actuators on IO only when the prearmed flag is set.
    (Same behaviour as FMU).

fixes #12457 #12713

@dagar
Copy link
Member

dagar commented Aug 25, 2019

This looks good, although we need to decide if the default behavior should change.

@jlecoeur jlecoeur force-pushed the pr-cbrk_prearmed_en branch from 1b3dda8 to 49e5d75 Compare August 25, 2019 14:48
@jlecoeur jlecoeur changed the title Add param CBRK_PREARMED_EN Add param COM_PREARM_MODE Aug 25, 2019
Condition to enter the prearmed state, an intermediate state between disarmed and armed
 * in which non-throttling actuators are active.
 *
 * @value 0 Disabled
 * @value 1 Safety button
 * @value 2 Always
@jlecoeur jlecoeur force-pushed the pr-cbrk_prearmed_en branch from 49e5d75 to 755f764 Compare August 25, 2019 14:51
@jlecoeur
Copy link
Contributor Author

Modified to keep same behaviour for safety button users, and make the system safer by default for the others (with the option to opt-in for the less safe behaviour)

@Antiheavy
Copy link
Contributor

@jlecoeur can you help translate for me how this could affect our use case scenario?

Our use case as follows:

  • Fixed Wing
  • No safety button
  • No RC input (no manual)
  • upon boot system goes into Manual mode, servos are energized at neutral, but do not move
  • upon solid GPS lock (and Home position established), the system automatically switches to Hold mode (i believe because no RC input?) and servos start stabilizing (rate damping?).

We like the above behavior because 1) servos are energized, but not moving around during indoor handling, 2) Servos start moving is an easy indicator of "ready" in outdoor handling and used in our preflight checklist.

@jlecoeur
Copy link
Contributor Author

@Antiheavy you will keep the same behaviour by setting COM_PREARM_MODE to ALWAYS.

@Antiheavy
Copy link
Contributor

@jlecoeur okay thanks.
FYI @Kjkinney

@dagar dagar requested a review from a team August 26, 2019 15:46
@dannyfpv
Copy link

teste on pixhawk4 v5 f-450
Modes Tested
Position Mode: Good.
Altitude Mode: Good.
Stabilized Mode: Good.
Mission Plan Mode (Automated): Good.

log:
https://review.px4.io/plot_app?log=60990dfd-0b59-4144-bf09-d2a574e41452

teste on pixhawk1 v2 fantom wing
Modes Tested
Altitude Mode: no rc response th, ail, elev
Stabilized Mode: Good.
Mission Plan Mode (Automated): Good.

log:
https://review.px4.io/plot_app?log=fae7a3ed-93e2-4fbe-af5e-2b411efad1bf

@jorge789
Copy link

Tested on PixRacer V4:
Modes Tested

Position Mode: Good.
Altitude Mode: Good.
Stabilized Mode: Good.
Mission Plan Mode (Automated): Good.
RTL: Good.

- Procedure
Arm and Take off in position mode, after flying for approximately one minute, switched to altitude then stabilized mode proceed to switch to mission plan mode then make sure that vehicle follows all waypoints as shown in QGC, once completed all waypoin activate RTL and see landing behaviour.

Notes:
No issues noted, good flight in general.

Log:
https://review.px4.io/plot_app?log=d0d66690-64d7-4857-befc-4af5aac8eb5b

Tested on CUAC+ V5:
Modes Tested

Position Mode: Good.
Altitude Mode: Good.
Stabilized Mode: Good.
Mission Plan Mode (Automated): Good.
RTL: Good.

- Procedure
Arm and Take off in position mode, after flying for approximately one minute, switched to altitude then stabilized mode proceed to switch to mission plan mode then make sure that vehicle follows all waypoints as shown in QGC, once completed all waypoin activate RTL and see landing behaviour.

Notes:
No issues noted, good flight in general.

Log:
https://review.px4.io/plot_app?log=9a6c859e-5974-45ca-b451-7b67c0571c81

@Junkim3DR
Copy link

Tested on Pixhawk 4Mini v5

Modes Tested

  • Position Mode: Good.
  • Altitude Mode: Good.
  • Stabilized Mode: Good.
  • Mission Plan Mode (Automated): Good.
  • RTL: Good.

Procedure
Arm and Take off in position mode, after flying for approximately one minute, switched to altitude then stabilized mode proceed to switch to mission plan mode then make sure that vehicle follows all waypoints as shown in QGC, once completed all waypoint activate RTL and see landing behaviour.

Notes:
No issues noted, good flight in general.

Log:
https://review.px4.io/plot_app?log=60549622-b259-45fd-92c2-ba88dc616bdd

Tested on NXP FMUK66 v3

Modes Tested

  • Position Mode: Good.
  • Altitude Mode: Good.
  • Stabilized Mode: Good.
  • Mission Plan Mode (Automated): Good.
  • RTL: Good.

Procedure
Arm and Take off in position mode, after flying for approximately one minute, switched to altitude then stabilized mode proceed to switch to mission plan mode then make sure that vehicle follows all waypoints as shown in QGC, once completed all waypoint activate RTL and see landing behaviour.

Notes:
No issues noted, good flight in general.

Log:
https://review.px4.io/plot_app?log=0bfad336-a490-417f-a837-79d25142743e

* Condition to enter prearmed mode
*
* Condition to enter the prearmed state, an intermediate state between disarmed and armed
* in which non-throttling actuators are active.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is a non-throttling activator?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A non throttling actuator is an actuator that does not drive a motor/engine. The underlying assumption is that throttling actuator is dangerous and should not move until the system is armed, while non-throttling actuators (ailerons, flaps, camera gimbal, etc) are safer and can move before the system is armed.

Right now, throttling actuator is defined here (NaN means the actuator will stay in disarmed position) https://github.com/PX4/Firmware/blob/a20b508d7e60a1332e3c9ff18d69df4d5e04eb5d/src/drivers/px4fmu/fmu.cpp#L1062-L1070. But I would not bother documenting that too precisely as this is likely to change when we introduce metadata at the mixer level to indicate which output is safe to prearm.

* Condition to enter the prearmed state, an intermediate state between disarmed and armed
* in which non-throttling actuators are active.
*
* @value 0 Disabled
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So would I be correct in saying

  • Disabled means that this mode doesn't exist - if you try and arm you just go from disarmed to armed.
  • Safety button means that you enter this mode when you press the safety button. How then do you move forward to armed?
  • Always means that you just go to this mode.

Does this require any docs/mention other than in the param? Frankly I don't understand what it is for, how you move to armed from this state, what an end user needs to do, or when they would set it.

Copy link
Contributor Author

@jlecoeur jlecoeur Aug 28, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You are correct.

How then do you move forward to armed?

As usual: arm switch, or arm stick motion, or MAVLink command

Does this require any docs/mention other than in the param?

Frankly I think some doc is needed here.
In fact, I was not aware of this state before I loaded a custom mixer on an airframe with the safety switch disabled (CBRK_IO_SAFETY), and the actuators started moving right after plugging the battery! This is now fixed with this PR: if you disable the safety switch the system will not prearm on its own by default.

From the user perspective, "prearmed" means "my ailerons move but my propellers do not spin".

In the default case of a system with safety button (COM_PREARM_MODE=Safety button and CBRK_IO_SAFETY=0), the behaviour is

  • power up
  • all actuators are locked into disarmed position, it is not possible to arm
  • safety switch is pressed. The system is now prearmed: non-throttling actuators start moving. System safety is off: it is now possible to arm
  • arm command is issued: The system is armed: all actuators move

Now the non default cases:
COM_PREARM_MODE=Disabled:

  • power up
  • all actuators are locked into disarmed position, it is not possible to arm
  • safety switch is pressed. All actuators stay locked into disarmed position. System safety is off: it is now possible to arm
  • arm command is issued: The system is armed: all actuators move

COM_PREARM_MODE=Always:

  • power up
  • The system is now prearmed: non-throttling actuators start moving. Safety is on: it is not possible to arm
  • safety switch is pressed. System safety is off: it is now possible to arm
  • arm command is issued: The system is armed: all actuators move

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And now for systems without safety button.

COM_PREARM_MODE=Safety button and CBRK_IO_SAFETY is engaged:

  • power up
  • all actuators are locked into disarmed position. System safety is off: it is possible to arm.
  • arm command is issued: The system is armed: all actuators move

COM_PREARM_MODE=Disabled and CBRK_IO_SAFETY is engaged:
Same as above

COM_PREARM_MODE=Always and CBRK_IO_SAFETY is engaged:

  • power up
  • The system is now prearmed: non-throttling actuators start moving. Safety is off: it is possible to arm.
  • arm command is issued: The system is armed: all actuators move

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you! I have created PX4/PX4-user_guide#567 to track this. I won't have time for it this week - feel free to find a place where you think docs should go if you need it in earlier.

Copy link
Member

@bresch bresch left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems good. I'm also in favor of keeping the current logic as default.

@dagar dagar merged commit 4735533 into master Sep 2, 2019
@dagar dagar deleted the pr-cbrk_prearmed_en branch September 2, 2019 14:47
@hamishwillee
Copy link
Contributor

@jlecoeur Can you please review docs for this: PX4/PX4-user_guide#569

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.

Add option to force disarmed values when CBRK_IO_SAFETY is set
9 participants