Skip to content

Conversation

@damosvil
Copy link
Contributor

@damosvil damosvil commented Feb 14, 2023

This pr uses a 4th pwm setting value DYNAMIC besides, 24, 48, and 96 to implement dynamic pwm frequency. Thresholds are configurable, but the default ones are:

  • up to 40% throttle 96khz
  • up to 60% throttle 48khz
  • otherwise 24khz

Demag and stall events notification flag cleared during dshot configuration

Fixed 640ms scheduler step

Updated status frame to carry demag metric max, and demag, desync and stall event flags

Fixed stall bug

Added clear for demag metrix max

Scheduler has been rewritten and optimized to allow sending demag metric every 64ms and temperature, status, debug1 and debug2 every 256ms

Demag_Detected_Metric_Max scaled to take full advantage of 4bit, small optimization in temperature PWM limit to follow Temp_Pwm_Level_Setpoint

Improved comment in stat frame

Fixed max demag event load for the case it is below 120 (should happen only when motors are stopped)

Clear notification flags after sending status frame

Updated .gitignore

Fixed review findings

Rebased to develop

Set default power rating to 2S+
Fixed calculate_pwm_bits

Fixed calculate_pwm_bits function

Fixed some compiling errors

Fixed format

Fixed whitespaces

Merged latest edt_events changes

Optimized t1_int pwm bitN code paths
@damosvil damosvil self-assigned this Feb 14, 2023
@damosvil damosvil changed the title Initial variable pwm implementation Variable pwm Feb 14, 2023
@stylesuxx
Copy link
Contributor

stylesuxx commented Feb 14, 2023

This is pretty awesome! I think we should rename this to "dynamic" instead of variable - what do you think?

We have variable PWM that can be set to either a fixed value (24, 48, 96) or dynamic.

What I am also wondering is: How did you arrive at those thresholds? You think it would make sense to allow the user to set those themselves?
I am particularly thinking about people here who use throttle limits in their FC firmware. This would mean, that they might never go above 60% throttle and thus not benefit from the dynamics at all...

@damosvil damosvil changed the title Variable pwm Dynamic pwm Feb 14, 2023
@damosvil
Copy link
Contributor Author

@stylesuxx
I have thinking on these thresholds based on my experience, but I would also like the idea of allow the user to decide.
Should I add then 2 more settings, at the end of the existing ones?

@stylesuxx
Copy link
Contributor

I think it would make sense to give the users a way to tinker with it a bit - maybe we decide later on, that there is a perfect value, but for starting out, let's add two more settings.

We should also consider the side effects though - can we properly safeguard it? I don't want to run issues that might be dangerous because the user set some "stupid" value...

@damosvil
Copy link
Contributor Author

damosvil commented Feb 14, 2023

I think that we could allow them to choose between [20-60] for the 96/48 low throttle threshold (Thres1) and [30-90] for the 48/24 high throttle threshold (Thres2), always constraining them to Thres1 + 10% <= Thres2, or something like that, allowing them to choose in steps of 5 or 10%. What do you think?

@howels
Copy link

howels commented Feb 15, 2023

With AM32 the concept of linking PWM to RPM instead of throttle was introduced, and seems to be preferred as a means of dynamic PWM. Should this be considered here?

@stylesuxx
Copy link
Contributor

stylesuxx commented Feb 15, 2023

AM32 has more means to set eRPM into perspective (and actually get RPM) - kv, pole count and such - Bluejay does not have this - at least not yet - I am not entirely sure if it would make sense to couple this to eRPM. But it is definitive something worth investigating.

Right now this is purely a POC with unclear outcome. We have also discussed more discrete scaling, not just in steps. But the steps and throttle values would be a first step to play around with things and see if it's worth the effort.

DISCLAIMER: should you attempt to test this, please only do so with a current limited (200mA) power supply for now.

@damosvil damosvil marked this pull request as ready for review February 15, 2023 17:34
@damosvil
Copy link
Contributor Author

@damosvil damosvil force-pushed the variable_pwm branch 3 times, most recently from 5885f6d to a19db5c Compare February 16, 2023 18:11
@damosvil damosvil changed the base branch from pwm_setting to develop February 16, 2023 18:12
@damosvil
Copy link
Contributor Author

I made more flights abusing pretty hard my Meteor 65 and extracted several logs.
There were no problems and no smoke.
Please, use this blackbox viewer branch if you want to inspect the logs (there you can find a compiled version):
betaflight/blackbox-log-viewer#625

DYNAMIC_PWM_BTFL_BLACKBOX_LOG_Meteor65_20230216_190311.zip

@damosvil
Copy link
Contributor Author

More testing on a 5 inch kwad. Betaflightf4 target with 4 individual LANRC 32A 3-6S ESCs:
BTFL_BLACKBOX_LOG_Galaga_20230220_00.zip

Added missing bits to update PWM cycle length

Fixed several bugs

Added low and hight variable pwm rc pulse thresholds

Fixed some bugs

Added DYNAMIC pwm option to build.sh script and Makefile

Bumped EEPROM layout revision to 207
@github-actions
Copy link

Build artifacts:

@damosvil
Copy link
Contributor Author

damosvil commented Mar 4, 2023

Tests flawed because of flashing with old esc-configurator tool so no dynamic pwm has been tested. This pr has been superseeded by #73. Cancelled

@damosvil damosvil closed this Mar 4, 2023
@stylesuxx stylesuxx deleted the variable_pwm branch July 23, 2024 10:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants