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

Reset max stress level metric to minimum #216

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from

Conversation

vmzhivetyev
Copy link

@vmzhivetyev vmzhivetyev commented Sep 12, 2024

Current Behavior

The Demag_Detected_Metric_Max currently stores the maximum value observed since the ESC was armed. This results in the metric becoming static after reaching its peak, never decreasing until the ESC is disarmed.

Proposed Change

Reset the Demag_Detected_Metric_Max metric each time a STATUS telemetry packet is sent. So that the value in a STATUS telemetry packet would represent the Max stress level in the last second (according to current scheduler implementation).

Rationale

  • Offload the logic of processing the max stress level to the FC (e.g., Betaflight), allowing for more sophisticated and/or different modes of analysis.
  • FC side alarms will be able to activate only when stress is genuinely high, rather than remaining permanently active after the threshold is exceeded.
  • Make the behavior consistent with Demag/Desync/Stall flags, which are also part of the STATUS EDT packet.
  • Speaking of blackbox logs, such change would provide meaningful data throughout the entire flight, instead of only capturing information before the first high-stress situation like a snap roll.

Tests

This is what 40 seconds of blackbox log look like if recorded with this change.
Screenshot 2024-09-13 at 00 34 28

Copy link

Build artifacts:

@stylesuxx stylesuxx added the enhancement New feature or request label Sep 12, 2024
Copy link
Contributor

@stylesuxx stylesuxx left a comment

Choose a reason for hiding this comment

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

Thank you for the comprehensive PR, makes sense in regards to making the behavior consistent.

I think it'd be important to adjust the EDT specs accordingly, since it's not specified in which timeframe context this status frame value is to be seen.

Ping @AlkaMotors - how do you handle the max stress level in the EDT status frame in AM32 - AFAIK you have that implemented too, no?

src/Modules/Scheduler.asm Outdated Show resolved Hide resolved
@vmzhivetyev vmzhivetyev force-pushed the update-max-stress-level branch from e99d053 to 554d016 Compare September 13, 2024 01:35
Copy link

Build artifacts:

@stylesuxx stylesuxx added this to the v0.22.0 milestone Sep 13, 2024
Copy link
Contributor

@hyp0dermik-code hyp0dermik-code left a comment

Choose a reason for hiding this comment

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

LGTM

hyp0dermik-code added a commit to hyp0dermik-code/extended-dshot-telemetry that referenced this pull request Sep 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
don't merge enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants