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

Improved Tumble Detection and Handling #481

Merged
merged 1 commit into from
Oct 4, 2019

Conversation

whoenig
Copy link
Contributor

@whoenig whoenig commented Oct 4, 2019

This change improves several issues with the current tumble
detector:

  1. The old detector was only enabled if the Kalman filter
    was initially enabled. If one switched dynamically to a
    different state estimator, this important safety feature did
    not engage.
  2. The old detector triggered by angle, which is an estimated
    value. If the state estimator diverged, no tumble was detected.
    The new solution relies on the accelerometer instead, a direct
    sensor measurement.
  3. The old detector was not sticky, i.e., a user might pick
    up a CF and it starts spinning again, see issue Crash/tumble detection should not re-enable motors when flipped back #258. The new
    version triggers an emergencyStop instead. The emergency stop
    can be reset and read using the new parameter stabilizer.stop.
  4. Smaller fixes:
    a) Add more #ifdef guards to only compile actually enabled code.
    b) Interface clean-ups

This replaces the PR #470. This change has been tested on a bench
test and with the Crazyswarm.

This change improves several issues with the current tumble
detector:
  1. The old detector was only enabled if the Kalman filter
     was initially enabled. If one switched dynamically to a
     different state estimator, this important safety feature did
     not engage.
  2. The old detector triggered by angle, which is an estimated
     value. If the state estimator diverged, no tumble was detected.
     The new solution relies on the accelerometer instead, a direct
     sensor measurement.
  3. The old detector was not sticky, i.e., a user might pick
     up a CF and it starts spinning again, see issue bitcraze#258. The new
     version triggers an emergencyStop instead. The emergency stop
     can be reset and read using the new parameter stabilizer.stop.
  4. Smaller fixes:
     a) Add more #ifdef guards to only compile actually enabled code.
     b) Interface clean-ups

This replaces the PR bitcraze#470. This change has been tested on a bench
test and with the Crazyswarm.
@krichardsson krichardsson merged commit febc780 into bitcraze:master Oct 4, 2019
@krichardsson
Copy link
Contributor

Thanks!

@krichardsson
Copy link
Contributor

Just a comment on adding emergencyStop as a parameter
PARAM_ADD(PARAM_UINT8, stop, &emergencyStop)

I think this might be a bit on the edge, @ataffanel what do you think?

Originally the concept was that parameters should only be updated from the client and that the underlying variable must never be updated by the firmware.
At some point we started to use "triggers" which is boolean parameters that can be set from the client and reset by the firmware.

The emergencyStop is more of a state variable that now is exposed as a parameter.

The main reason to restrict the use of parameters is mainly that there is no feedback to the client and thus the cached parameters in the client are invalid.

@ntamas
Copy link
Contributor

ntamas commented Oct 4, 2019

My two cents: how about officially declaring some of the parameters as "volatile" in the sense that clients are not allowed to cache it and must retrieve a new value every time? This could even be provided as a binary flag in the TOC so clients would not need to hard-code the set of volatile parameter names.

@whoenig
Copy link
Contributor Author

whoenig commented Oct 4, 2019

That's a good point. For this specific use-case we could also:

a) Add another CRTP packet to reset the emergency, similar to https://github.com/bitcraze/crazyflie-firmware/blob/master/src/modules/src/crtp_localization_service.c#L158-L161; or
b) Use a similar trigger logic to reset the emergency as we use for resetting the Kalman filter.

In both cases we could expose the current state as logging variable, or use DEBUG_PRINT to at least warn a user if emergencyStop was triggered.

@whoenig whoenig deleted the bugfix_issue258 branch October 4, 2019 19:01
@krichardsson
Copy link
Contributor

I vote for b and I think a DEBUG_PRINT would be nice

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.

3 participants