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

PID Rework #1043

Merged
merged 26 commits into from
Sep 17, 2021
Merged

PID Rework #1043

merged 26 commits into from
Sep 17, 2021

Conversation

Ralim
Copy link
Owner

@Ralim Ralim commented Sep 14, 2021

Please try and fill out this template where possible, not all fields are required and can be removed.

  • Please check if the PR fulfills these requirements
  • The changes have been tested locally
  • There are no breaking changes
  • What kind of change does this PR introduce?
    Lots of PID rework, basically taking the ideas from Major issue in TS100 code - Chapter 1. ADC #1038 and also giving a lot of stuff a once over

  • Cleaned up timings to be 10/5Hz update rates

  • Cleaned up a second pass on making sure adc is only run by pid loop

  • What is the current behavior?

  • Other information:

Probably need to re-check all devices are correctly reaching 100% max power draw that they can reach.
Seems to be working here on my tester units but need to do more testing before merge

  • Suspect may be adc timing quirks with QC3 code, will need to check this in more depth
  • need to run this on on bench supply to check stability at all voltages
  • Still seeing a little overshoot at higher voltages at first but stable after that, suspect thats to do with the derating we have to handle thermal lag that may no longer be required (more testing to do)

@Firebie and @discip you may want to give this a try since I know your following along the issue and see what you make of it.

@discip
Copy link
Collaborator

discip commented Sep 14, 2021

@Ralim
Good afternoon Sir,
as you might have expected, I have already run some basic tests.

  1. Regarding the stability it is waaaaay better, but as you mentioned above, it overshoots by ~40°C before settling almost exactly at the set temperature (320°C).
    One more thing in the version you put up earlier is, that the temperature increase briefly halts at about 280°C only to overshoot as mentioned above.
  2. Temperature calibration behaves very strange:
    Currently it loops showing the dots until it is taken from the power supply or until the tip gets removed.
  3. The changes above a207f03 seem to have broken something in the way the reaching of ends of value ranges is handled.
    Because the following entries are looped through, when the A-button is held down, instead of pausing at the end of a value range.
Power settings Soldering settings Sleep mode User interface Advanced settings
*QC voltage Heat on power up Sleep timeout *Temperature unit *Detailed idle screen
PD timeout Temp change short Shutdown timeout Display orientation *Detailed solder screen
Temp change long Motion sensitivity *Cooldown blink Power pulse
Allow locking buttons *Scrolling speed Power pulse delay
*Reverse +- keys Power pulse duration
Anim. speed
*Anim. loop
*Invert screen

Entries marked with a * consist of only two states / values.

setup
iron Firmware Version PCB Version Power Supply
TS80P 446faa4 2 UGREEN 65W GaN Charger

@Firebie
Copy link

Firebie commented Sep 14, 2021

Pinecil, B2 tip, QC charger 19V 33W.
Overshoot is around 10°C on initial heat up, then very stable readings around set temperature.

@discip
Copy link
Collaborator

discip commented Sep 14, 2021

@Ralim
Good evening Ben.
Prior to details:
I have executed a reset after upgrading the firmware.

Have tested the last build and I have to say, that

  1. the temp ramp-up improved in regard of overshoot, but behaves somewhat 'interesting' shortly before reaching set temp (320°C).

By the way: Why is the full potential (30 W) not used at all? Would this not speed up the heating? (chart for reference)

Here is a capture (23 s) of the temp ramp-up sequence:

DELETED due to false data resulting from bad tip!

  1. temperature calibration is still buggy (still dot looping [See item 2. in my previous post.]).
iron Firmware Version PCB Version Power Supply
TS80P 3d99251 2 UGREEN 65W GaN Charger

@sandmanRO
Copy link

Hello Gentlemen/Hello discip,
Very interesting the temperature chart you posted. There is definitely something odd going on. I see those temperature reading drops from +290C to +250C or so within one reading cycle. They can not be in real. If they would be higher (like when we read too early after the heating window) that would have had an explanation but not such temperature drops in excess of 40C. However, this might explain the severe overshoots, as each of these "drops" lead to un unnecessary extra-charge of the pid integrator (a low temperature would be seen as a large setpoint delta that in turn would update the integrator with a larger than necessary value). And yet, if there would be a bug in the reading routine we would expect to see the same temperature reading drops after reaching the setpoint, right? But we don't see that so most likely something else is going on. The only significant difference would be that after reaching the setpoint the power demand drops and that puts less stress on the power supply / power circuitry (tip included). I wonder if you have a similar chart with side-by-side temperature-voltage-power readings. Mind if I'm only speculating as I do not have the hardware you are using, but my suspicions are pointed towards the power supply setup. All I can say for sure, that pid integator was tested on four different TS100 with all 10 designs miniware tips (that go form needle like to the "massive" horseshoe like tips) and I rarely seen overshoots in excess of 1-2 Celsius in the 4 (injected) x 16 averaged temperature value...definitely nothing even near close to the 10-40 degrees overshoots you are experiencing.

@discip
Copy link
Collaborator

discip commented Sep 14, 2021

@sandmanRO
First of all thank you for looking into this.

  1. I don't know, If I got you right about the chart, but I think the parameters you are looking for are already there, but in a rather cryptic depiction.
    I left the middle row empty because there are only 3 occurrences of the voltage being not 12.1 V but 12.2 V.
    I thought it would be obvious the way I put it.
    In the caption of the middle row it states:

V (if no value 12.1)

Since I am only testing (regular user, no programming), you have to approach @Ralim to discuss the hows and whys of the coding.

kind regards

@sandmanRO
Copy link

...you are right, discip, my bad...it should have been obvious to anybody. I should go to bed I suppose (here is 2:38AM).

@Firebie
Copy link

Firebie commented Sep 15, 2021

For the calibration issue:

index 1d5c2821..c4ffa8c3 100644
--- i/source/Core/Src/gui.cpp
+++ w/source/Core/Src/gui.cpp
@@ -641,7 +641,7 @@ static void setTipOffset() {
       OLED::refresh();
       osDelay(100);
     }
-    setoffset = TipThermoModel::convertTipRawADCTouV(offset / 16);
+    setoffset = TipThermoModel::convertTipRawADCTouV(offset / 16, true);
   }
   setSettingValue(SettingsOptions::CalibrationOffset, setoffset);
   OLED::clearScreen();

@discip
Copy link
Collaborator

discip commented Sep 15, 2021

@Firebie
Good morning Sir,

  1. are you experiencing the same behavior too?
  2. For some reason I am not able to find the code snippet you posted. 🤷‍♂️
    So would you please tell me, where I have to change these lines, to test if this works for me? 🥺

thanks in advance

@Ralim
Copy link
Owner Author

Ralim commented Sep 15, 2021

Ah looks like my late night ticket writing wasn't quite up to scratch.
I'm aware of the overshoot and have some theories (just ran out of time to test them), will be poking them a little more tonight as I get time.

Thanks @Firebie , will need to figure out why that broke.

@discip

In regards to your power table, can you confirm you are measuring those values external to the iron?
Very close to the full 30W should be used by the iron.
Those menu changes will be from the settings rework, ill give them a poke too

Otherwise still trying to figure out why we are getting the little overshoot at the end.

@Firebie
Copy link

Firebie commented Sep 15, 2021

Hi @discip
It is in pid branch, you need to find piece 'setoffset = TipThermoModel::convertTipRawADCTouV(offset / 16);' in 'gui.cpp'
I have not tried to reproduce it, but your behavior is possible, especially if you get offset less then default.

@Ralim
Copy link
Owner Author

Ralim commented Sep 15, 2021

Okay PID overshoot should be better now.. Well it is for me 😓
Going to look at the menu now

@sandmanRO
Copy link

If I may suggest an additional change that would improve the after setpoint behavior...once we exceed the setpoint, the delta temperature becomes negative. However, the micro volts - temperature lookup table does not handle negative values so it would return zero instead of a negative value. This would delay the reaction of the PID after setpoint as it would rely only on power store decay rather than both decay and active update. In my implementation I added to the Hakko lookup table something like this:
static const int16_t uVtoCTable[] = {
-1244, -50,
-1010, -40,
-770, -30,
-522, -20,
-266, -10,
0, 0,
266, 10,
522, 20,
...
}

@discip
Copy link
Collaborator

discip commented Sep 15, 2021

@sandmanRO
Could you upload a video showing the heat up process?

@sandmanRO
Copy link

Ok, just give me a minute to grab a camera and improvise a video fixture.

@Ralim
Copy link
Owner Author

Ralim commented Sep 15, 2021

Hmm interesting that the time difference is that big.
Wonder if this should be some form of setting to account for tip size.

In my testing both are similar enough I'm happy to go with a gain of 2 if it makes that much of a difference for a large tip :)

@sandmanRO
Copy link

Now the big question...how do I upload a video here?...I tried as zip file but github complained that "is not included in the list"

@sandmanRO
Copy link

I see...they accept MOV, MP4 not GP...let me try to convert it.

@sandmanRO
Copy link

This is ridiculous...as I converted to MP4 now they complain it is too big and it should be under 10MB while the file is 14MB. Let's see what I can do.

@discip
Copy link
Collaborator

discip commented Sep 15, 2021

@Ralim
Just tested the gain=2 build.
I think gain=1 is better in regard of overshoot and speed (21 s).
Will have to reflash the gain=2 build and test again.

@discip
Copy link
Collaborator

discip commented Sep 15, 2021

@sandmanRO
Maybe convert to gif.

@Ralim
Copy link
Owner Author

Ralim commented Sep 15, 2021

Ah heck, yeah it's a bit messy. Compress it as much as you can or throw it on a different host if you need to.

If that's too messy you can also throw it to my email inbox (up to 25MB) and I can rehost it here

I'm easy to find at ralim@ ralimtek.com

@sandmanRO
Copy link

Ok...I will send to your email as mpg as the smallest format I have available (about 5MB...of course this format is not accepted by github)

@Ralim
Copy link
Owner Author

Ralim commented Sep 15, 2021

@discip definitely keen to hear your thoughts on the two builds.
This could also be something we tweak for each model (mildly suspect MHP30 will need further tweaking, since it's a bit odd compared to the rest).

@discip
Copy link
Collaborator

discip commented Sep 15, 2021

@Ralim
My guess seems to be right since gain=1 took only 18 s.
So please revert to gain=1 again. 😊

One more thing, which I just now stumbled over, is that the real temperature (external measurement) is off by ~15°C.

@sandmanRO
Copy link

I sent the email like 10 minutes ago. @discip, you sure you did not mix-up the builds? You get shorter to setpoint time with gain=1 as opposed to longer to reach setpoint time with gain=2? That does not make any sense.

@discip
Copy link
Collaborator

discip commented Sep 15, 2021

@sandmanRO
I am sure, because with gain=2 the temperature overshoots by almost 30°C and after that dangles around set temp (320°C) much longer than with gain=1.
But will redo my test just in case. 😅

@sandmanRO
Copy link

I see...so you are counting for the time the temperature stabilizes around setpoint not when the temperature reaches / exceeds the setpoint.

@Ralim
Copy link
Owner Author

Ralim commented Sep 15, 2021

Video is still processing (whatever that really means) but should appear here at some point soon : https://youtu.be/O68aObWneFk @discip

@sandmanRO
Copy link

I feel like I'm missing the point. With the video I sent, you will see that with gain=1 the PID is actually "struggling" to reach the setpoint (the PID spends un unnecessarily long time to cope with the remaining 2-3C)

@sandmanRO
Copy link

It gets there eventually but much slower than it could. Now I'm curious what could be so different at hardware level that would lead to such totally opposed results.

@sandmanRO
Copy link

Not that is it really matters. As Ralim pointed out earlier, it's just a pure academic curiosity.

@discip
Copy link
Collaborator

discip commented Sep 15, 2021

@Ralim & @sandmanRO
Sorry just realized my tip (TS-J02) seems to be faulty, as the results with 2 other ones (TS-K4 & TS-B02) are much, much better.
I was some kind of chocked by the steadiness of the temp ramp-up on @sandmanROs iron.

Will test the gain=2 build as well and report back accordingly.

TS-J02.mp4
TS_B02.mp4
TS-K04.mp4

EDIT:
Are you able to watch the videos? I can't!

@sandmanRO
Copy link

sandmanRO commented Sep 15, 2021

@discip, please humor me. Could you please also check that you are using the very latest update that Ralim put out (the one that includes the limits on PID?). Your earlier results just throwed my brains into the bushes so please help me get them back.
P.S. I'm just looking at videos you just posted (for some reason it would let me see only one out of 3, that is this one 133452176-982bd8f7-f03f-4364-851f-0497338bbe2f,mov). The temperature evolution looks just fine to me, with smooth setpoint reach. Don't you agree?

@sandmanRO
Copy link

Now I can see them all. Indeed, that TS-J02 looks bad... definitely something is not right with that tip. Just don't put it aside though. If it's brand new, apparently the brand new tips are "temperamental". I recall when we purchased the TS100s, each unit came with a collection of 10 tips. At the first run it was scary... with something like +30 out of 40 tips, the temperature readings were jumping all over the place. Luckily the seller advised us to run the tips for a while at +300C then cool them off and retry. It's almost like something needs to meltdown inside the tip in order for it to work correctly afterwards (this is just a supposition as I could not find out how the inner connections between the connector, heater and thermcouple are done inside the tip).

@discip
Copy link
Collaborator

discip commented Sep 15, 2021

@Ralim & @sandmanRO
I have tested the gain=2 build and it works, but no significant improvement.
If higher mass tips are served better with this, than this should stay.

Thanks for the effort! ❤

Since the temp ramp-up chart resulted from the bad tip, I am going to remove it, to not confuse people reading this.

@Ralim
If no one has to complain about anything anymore, than this should be merged.
I am planning on changing the animation for the Power settings as well, maybe today.

@Ralim
Copy link
Owner Author

Ralim commented Sep 17, 2021

@sandmanRO
That is an excellent note that tips need a run-in time. Doesn't surprise me given how they are manufactured.
In terms of construction, online there are deconstructions of the TS100 / Hakko T12 ones. Havent found ones for TS80 yet though.

Also very interesting to catch up on this. I'm thinking I'll merge it in since it seems to be working with the gain of 2.
I also agree its a little academic, but it never hurts to discuss :)

@Ralim Ralim merged commit 1d64e78 into master Sep 17, 2021
@Ralim Ralim deleted the ralim/pid branch September 17, 2021 08:53
@sandmanRO
Copy link

Hello Ralim, definitely it's a pleasure to see our minds interacting in real time. In a sense, the process is like a PID. We oscillate, we tune, sometimes we go baloney and overshoot....and yet, eventually we reach the best solution our minds could come up with. It might not be THE BEST solution in absolute terms (assuming such a thing exists) but as long as we are honest with the nature, and listen what it tells us, it can not be too far.

@Ralim
Copy link
Owner Author

Ralim commented Sep 17, 2021

Yeah absolutely, its very pleasurable to be working through these problems. I think much like most controller systems we converge on the best solution known at the time... Then a random @sandmanRO comes along and disturbs the whole system with new information and a better solution is converged on 🙃 (That is meant as a compliment)

@sandmanRO
Copy link

Thank you, Sir! (I think I'm blushing right now)

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.

4 participants