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 power limits after initial run phase is done #166

Merged
merged 1 commit into from
Sep 12, 2023

Conversation

damosvil
Copy link
Contributor

This pr moves the release of startup power limits until the end of the initial run phase:
imagen

@damosvil damosvil self-assigned this Aug 21, 2023
@damosvil damosvil added the bug Something isn't working label Aug 21, 2023
@damosvil damosvil added this to the v0.20.1 milestone Aug 21, 2023
@damosvil damosvil linked an issue Aug 21, 2023 that may be closed by this pull request
@damosvil damosvil force-pushed the bugfix/enforce_max_startup_power_3 branch from 0ade1e8 to 09f1b32 Compare August 22, 2023 16:22
@damosvil
Copy link
Contributor Author

damosvil commented Aug 22, 2023

Tested in the workbench jamming the motor.
In previous commit if throttle is raised during startup the last kick was done at throttle value, not limited.
In this last commit if the motor is jammed max startup power is respected unless the motor starts spinning. In that case applies power rampup implemented in set_pwm_limit_function.

Please, when possible could you check on your side?

@damosvil
Copy link
Contributor Author

damosvil commented Aug 27, 2023

Tested in workbench. Power supply over current protection never triggers.
Tested also crashing with a drone (motor 1 shaft was damaged so it didn't spin well):
https://youtu.be/HBaYtLEnpSM
https://youtu.be/6kz2UXlnqHg
https://youtu.be/VXC0j5dCzlE
https://youtu.be/WyG72G9rX_g

No magic smoke, in any motor neither in any esc.

@stylesuxx
Copy link
Contributor

Alright, painful to watch, but looking good in general. I did some testing with whoops, although not with the latest push. Did you do any turtle mode tests with 5" by any chance? Also was this with dyn idle on?

@damosvil
Copy link
Contributor Author

Tbh no dyn idle testing yet and no turtling. I will make more testing this week

@damosvil
Copy link
Contributor Author

More testing done. This time turtling:
https://youtu.be/WhxjvZtFrDQ
https://youtu.be/StgaBe4b51Q
https://youtu.be/8_GBsrCebOw
https://youtu.be/euY0VAXHNz4
https://youtu.be/07bvxammWyY

Dynamic idle also tested. Min/max startup power are 1010 and 1020 respectively. They are the bare minimum to start the motors. Above that the likelyhood of burning anything increases.
I realized that when dynamic idle is active the motors fight to keep the rpm when crashing and not in a proportional way, and the likelihood of burning the motors/esc increases.

@stylesuxx
Copy link
Contributor

Perfect, I added a note to the release notes for people to be careful when setting up min/max startup power.

I am wondering if we should adjust the min/max startup power defaults to a "save minimum". If not, we should probably also do a test run with the defaults, just to be on the save side.

@damosvil
Copy link
Contributor Author

damosvil commented Aug 29, 2023

In my opinion defaults are very high for 5 inch kwads (normally 1010/1020), even for 3 inch they may be too much on some ocasions. Unfortunatelly they should be adjusted to the absolute minimum for each type of kwad, depending on battery S, motor type and esc deadtime. Some sort of preset system or calculator may help, but I am not sure how far of scope may it be.
In addition some pilots may want to intentionally keep min/max startup power high, because they would want more power to turtle their kwads at the expense of burning them

@stylesuxx
Copy link
Contributor

Yeah, we could add a calculation to the wiki, I think that'd be out of scope for this issue. What I'd feel comfortable with is reducing min/max defaults, keeping 5" in mind. Worst case we would have people complaining about motors not spinning up or turtle mode not working properly - those requests would be easier to handle than people blowing up their shit because they did not read the release notes.

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.

OK, looks fine to me. We should still consider if we want to adjust the min/max startup power defaults - I would also need to adjust that in the confiurator then.

@damosvil
Copy link
Contributor Author

I think that we could set it to 1010/1020 for 5 inch kwads. I know that this is the most conservative option, but many people already know that they have to move sliders up to make it work on tinywhoops.

@stylesuxx
Copy link
Contributor

Yes, better to err on the side of caution. Let's adjust min/max defaults then.

@damosvil
Copy link
Contributor Author

damosvil commented Sep 8, 2023

Assets for 0.20.1 RC1 (testing)
v0.20.1_RC1.hex.zip

Remember to keep min/max startup power to the minimal possible value for maximum protection on crashes and set motor idle to keep motors spinning after startup.

@nerdCopter
Copy link

nerdCopter commented Sep 9, 2023

O_H_5_48_g93e662f.hex compiled by @hypOdermik Aug 31
1080 startup
1120 max
(pretty much lowest i could set for consistent startups / turtle)

BETAFPVF4SX1280
0802SE 19500kv, 1S, 9% motor idle, no rpm filter, no dynamic idle.

0.21.1 startup testing https://youtu.be/YXQBVlivhEk (thumbnail is settings)
0.19.2 1S flight https://youtu.be/Afy_1ygsy8Y
0.21.1 1S flight https://youtu.be/oaZx7F-ed3Q

@damosvil
Copy link
Contributor Author

damosvil commented Sep 9, 2023

BETAFPVF4SX1280 deadtime 5, 0702 28500kv motors & Bluejay 0.20.1
All motors start at 24khz, 48khz, 96khz: idle = 8% min/max startup power = 1040/1060

TMOTOR F411 AIO 1S 6A ELRS, deadtime 10, 0702 32000kv motors and Bluejay 0.20.1
at 24khz: idle 8%, min/max startup power = 1050/1070

@hyp0dermik-code
Copy link
Contributor

GEPRC-AIO-F722 G-H-30 1804 2450kV @ 6S Bluejay 0.20.1 from assets above
All motors consistently start at 1010/1020 min max. 8% static idle

@damosvil damosvil force-pushed the bugfix/enforce_max_startup_power_3 branch from 93e662f to 09f1b32 Compare September 11, 2023 05:36
Pwm_Limit_By_Rpm allowance removed from inital_run_phase_done to pwm limit by rpm will increase in set_pwm_limit function

Reenabled rampup during initial run phase
@damosvil damosvil force-pushed the bugfix/enforce_max_startup_power_3 branch from 09f1b32 to b7264ea Compare September 11, 2023 05:44
@damosvil
Copy link
Contributor Author

damosvil commented Sep 11, 2023

Assets for 0.20.1 RC2 with power rampup during initial run phase (testing)

0.20.1-rc2.zip

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.

Alright, looks fine to me.

I suggest we merge and move on to make version "v0.21.1 RC2 (for testing)" available via configurator.

Maybe it would make sense to move the test results regarding min/max startup power and idle to the Startup power wiki page as a reference?

@damosvil
Copy link
Contributor Author

Yes, please merge it.
I will update the wiki

@stylesuxx stylesuxx merged commit 04f01a9 into v0.20.1 Sep 12, 2023
@stylesuxx stylesuxx added fix and removed bug Something isn't working labels Sep 12, 2023
@stylesuxx stylesuxx deleted the bugfix/enforce_max_startup_power_3 branch July 23, 2024 10:09
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.

2 failed ESCs on bluejay 0.20.0
4 participants