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
[PoC] limit maximum power more efficiently #697
Conversation
workspace/TS100/Core/Src/power.cpp
Outdated
if (fastPWM && output == powerPWM) { | ||
fastPWM = false; | ||
totalPWM = 255 + 17; | ||
TIM2->ARR = totalPWM; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Accessing the timer directly should be pulled out to a function in /BSP/
as its not generic across devices/boards/microcontrollers 😢
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Guess why I added [PoC] to the beginning of the commit message? ;)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can try to implement this idea without layering violations and with more comments but I'd prefer to do that after your testing and evaluation for obvious reasons.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
With current "slow" PWM the power can be pumped up to ~93.8 % of time, with "fast" that's lowered to ~88.2 %. I'm not sure if those ~5.5 % are worth the complexity added. OTOH the code is minimal, so probably not much harm. What I wonder is whether the "fast" mode really fast enough for all reasonable usecases.
workspace/TS100/Core/Src/power.cpp
Outdated
@@ -24,6 +25,21 @@ int32_t tempToX10Watts(int32_t rawTemp) { | |||
|
|||
void setTipX10Watts(int32_t mw) { | |||
int32_t output = X10WattsToPWM(mw, 1); | |||
if (fastPWM && output == powerPWM) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is the purpose of this logic to run the PWM faster when we are doing the inital heat up to allow for finer controll and then slow down after this point in time?
(Would appreciate some form of comment block ahead of this as to why this approach was chosen :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The idea is that the faster we run PWM, the less averaging we require from the output of the PSUs. But we need 7 ms before each ADC measurement cycle, and few more ms to do the 4 measurements, and we need to do that after each PWM cycle. This whole ADC time is "wasted" because the tip doesn't receive power (and we want the tip to receive as much as possible in the power-unlimited case), so naturally we'd like to be able to do the ADC measurements less often (by having longer PWM cycles). So my idea is to switch to fast PWM when the conditions do not require anything close to full power.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Another idea is that we could totally "skip" every other ADC cycle during large heatup events to save the dead time, and when heating up large differences we are not doing a huge amount of logic in the PID (just saturating at the output).
Of course this may have implications in stability, but could be an interesting optimisation.
But yes I agree with this concept :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I had that another idea too but couldn't come up with a simple enough implementation as it would involve changing the PWM period every single cycle and probably other trickery and would affect power/pwm calculations etc, so I got too confused to try to implement it :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Absolutely agree it could be very confusing 😅
I'm happy to go with this implementation for now.
BTW, in case anyone is wondering why I'm adding a space before physical units, that's because of https://physics.nist.gov/cuu/Units/checklist.html |
I think if you just cleanup all timer operations for changing values to use calls behind BSP (so that other boards can be supported) then this should be fine :) |
On Tue, Sep 15, 2020 at 05:40:29AM -0700, Ben V. Brown wrote:
I think if you just cleanup all timer operations for changing values to use
calls behind BSP (so that other boards can be supported) then this should be
fine :)
Did you have a chance to verify this with different PSUs and iron
models? I am not yet sure the change is worthy because I've only seen
it helping in just one very specific case.
|
I have tried this on a few models & power adapters here with no regressions; though I dont have any psu's that "shut off" most of mine current limit. |
Hey Ben :)
On Wed, Sep 16, 2020 at 04:50:50PM -0700, Ben V. Brown wrote:
I have tried this on a few models & power adapters here with no regressions;
though I dont have any psu's that "shut off" most of mine current limit.
Not being an EE I naively assumed that you can imitate something close
enough by limiting current with a lab PSU and adding a suitably sized
capacitor at the output. I also thought you have one of those "more
picky" QC/PD powerbanks.
I still have doubts my solution is useful to anybody but me but it
complicates the code enough to hesitate adding it, IMHO.
|
So I own a bunch of power supplies but they are all linear basically so they just get massive voltage droop. I did find it helped a bit with TS80@12V on a power limited power bank, but even this one isnt that picky.
I feel like this could give us an advantage in some heatup situations as well, and if we can reduce "time to hot" that more than justifies it too. But of course, its entirely your choice what you would like to do :) |
On Fri, Sep 18, 2020 at 02:33:48AM -0700, Ben V. Brown wrote:
I still have doubts my solution is useful to anybody but me but it
complicates the code enough to hesitate adding it, IMHO.
I feel like this could give us an advantage in some heatup situations as well,
and if we can reduce "time to hot" that more than justifies it too.
I'm not sure my experimental code makes any difference in non-power
limited situations at all, what do you have in mind?
|
Checked your patch - it really works. Even without power limit. |
On Fri, Sep 18, 2020 at 11:35:33AM -0700, Firebie wrote:
Checked your patch - it really works. Even without power limit.
I have 24V 4A Kodac PSU (probably problematic), which switches off during
vanilla Ralim software, but works with your patch.
Thanks a lot for testing, that really encourages me to continue!
I'll try to prepare something mergeable soon.
|
With slight cleanup I'm definitely happy to get this merged in, thank you @Firebie for testing 😅 |
262b325
to
ea7fe63
Compare
…er intervals With this a TS-I tip is usable with a small netbook 19 V / 30 W PSU with power limit set to 40 W (38.9 W is reported during the heating up stage). Without this the device just reboots on attempt to turn on the heater (unless the power limit is set to 10 or even 5 W). This code doesn't affect maximum power available and allows up to 73 W when a beefy 24 V / 96 W PSU is used. Should be useful for all models, not just TS100. The fixed comments are based on calculations, not measurements! Fixes Ralim#693.
ea7fe63
to
30be5e0
Compare
Ping |
@paulfertser |
Apologies, was not notified you had re-pushed 😓
Meh, github :( I'll keep that in mind the next time!
|
Thank you for poking, looks great to me :) |
With this a TS-I tip is usable with a small netbook 19 V / 30 W PSU with
power limit set to 40 W (38.9 W is reported during the heating up
stage). Without this the device just reboots on attempt to turn on the
heater (unless the power limit is set to 10 or even 5 W).
This code doesn't affect maximum power available and allows up to 73 W
when a beefy 24 V / 96 W PSU is used.
Should be useful for all models, not just TS100.
The fixed comments are based on calculations, not measurements!
Supposed to fix #693.
Please try and fill out this template where possible, not all fields are required and can be removed.
What kind of change does this PR introduce?
(Bug fix, feature, docs update, ...)
What is the current behavior?
(You can also link to an open issue here)
What is the new behavior (if this is a feature change)?
Other information: