Ralim / IronOS

Open Source Soldering Iron firmware
https://ralim.github.io/IronOS/
GNU General Public License v3.0
7.24k stars 718 forks source link

Important!!! Dangerous error °C/°F #264

Closed fishingpro closed 6 years ago

fishingpro commented 6 years ago

If you change the unit of temperature, all the settings of the tempatures(absolute values) remain before. Thus, if you select fahrenheit and set the soldering tip temperature for any mode more than 450 degrees, and after that change the unit of measurement back to degrees Celsius, the soldering iron begins to heat the stump above the allowable values until the "Error tip(BadTipString)" error appears. I think this error is very dangerous both for the soldering iron and for the technique that they are soldering.

Possible solutions: 1) Remove fahrenheit - bad solution 2) When changing the unit of temperature, reset all the settings of the temperature in the default for this unit - a normal solution 3) When changing the unit of temperature, convert the values of ° C <-> ° F - a good solution

PS: At the time when the error BadTipString appeared - I thought the tip of the soldering iron came to an end:^(

Ralim commented 6 years ago

@fishingpro Please use the provided issue template when filing issues

This is a known issue, and I will add a note to the readme (it needs a re-write anyway).

The reason this has not changed is because I'm waiting on @Rockman18 To get back to us about #178 If they dont respond soon then I will most likely impliment option 2. If i could have it my way it would be option 1.

fishingpro commented 6 years ago

Please use the provided issue template when filing issues

sorry:) I try to make the project better, but the language barrier hinders me.

JohnEdwa commented 6 years ago

The easiest and safest way is to always use Celcius internally and convert them to Fahrenheit when displayed (casting 'CELCIUS * 18 + 325)/10' to int works well) , but that comes with the issue of arbitrary looking values (300 to 350C in 10C steps converts to 572F, 590F, 608F, 626F, 644F and 662F, for example). That way you can do whatever changes you want in the code and never have to worry about forgetting to set the correct limit or check for F.

The other proper way is to add a conversion from C to F when you swap the option. A lookup table is needed most likely because swapping C -> F -> C -> F -> C etc would make the values behave weirdly due to the rounding errors.

@ralim I vote for dropping both F and C and converting completely to Glorious Kelvin Master-Race :D

fishingpro commented 6 years ago

A lookup table is needed most likely because swapping C -> F -> C -> F -> C

this is certainly a EPIC-CRUTCH - but I would have done the same thing :)

ghost commented 6 years ago

The easiest and safest way is to always use Celcius internally and convert them to Fahrenheit when displayed (casting 'CELCIUS * 18 + 325)/10' to int works well) [...] That way you can do whatever changes you want in the code and never have to worry about forgetting to set the correct limit or check for F.

I think it is the most plausible solution in relation to the effort that other proposals would require.

fishingpro commented 6 years ago

int c2f[]=//50..450 °С discret 10 {120, 140, 160, 180, 190, 210, 230, 250, 270, 280, 300, 320, 340, 360, 370, 390, 410, 430, 450, 460, 480, 500, 520, 540, 550, 570, 590, 610, 630, 640, 660, 680, 700, 720, 730, 750, 770, 790, 810, 820, 840 } //Converting Celsius to Fahrenheit with rounding up to 10 f = c2f[(c-50)/10];

JohnEdwa commented 6 years ago

It has to have as many steps as the Celcius scale, or you have the same 'rounding' issue. In your first list, 340C/644F would need to either map to 620F or 660F, which are 326,6C and 348.9C, respectively, so when you convert back from F to C you can never get 340C, only either 330C or 350C. 320C in 608F, which would also map to 620F, etc.

fishingpro commented 6 years ago

It has to have as many steps as the Celcius scale, or you have the same 'rounding' issue. In your first list, 340C/644F would need to either map to 620F or 660F, which are 326,6C and 348.9C, respectively, so when you convert back from F to C you can never get 340C, only either 330C or 350C. 320C in 608F, which would also map to 620F, etc.

the above array allows you to get the same number of steps and the values will be rounded. Minuses: 1) the error is 0..4 ° F 2) the increment is not always the same

fishingpro commented 6 years ago

No matter how you implement it, it would be nice to add somewhere protection from the possibility of heating the soldering tip above 500 ° C. Today I myself have experienced the danger of this situation;)

JohnEdwa commented 6 years ago

Ah, yeah, the site didn't show your edit so it was a comment to your 25F per step list.

One possible option would be rounding to nearest 5 as well, which would reduce the error to+-2: 120, 140, 160, 175, 195, 210, 230, 250, 265, 285, 300, 320, 340, 355, 375, 390, 410, 430, 445, 465, 480, 500, 520, 535, 555, 570, 590, 610, 625, 645, 660, 680, 700, 715, 735, 750, 770, 790, 805, 825, 840

Kulturnilpferd commented 6 years ago

By the whay how hot did your tips get by this error? Are the tips still usable if they get above 450C°? I think about a modded test version with temperatures above 450C° but doesn't want to break anything in my iron ^^

fishingpro commented 6 years ago

One possible option would be rounding to nearest 5 as well, which would reduce the error to+-2: 120, 140, 160, 175, 195, 210, 230, 250, 265, 285, 300, 320, 340, 355, 375, 390, 410, 430, 445, 465, 480, 500, 520, 535, 555, 570, 590, 610, 625, 645, 660, 680, 700, 715, 735, 750, 770, 790, 805, 825, 840

It seems to me to reduce the discreteness to 5 is not very good.

  1. Even 10 is not very convenient - long tune
  2. The error of 0..4F is not as great as it seems - it is slightly more than 2C, and this is clearly less than the intrinsic error of the temperature measurement method.
JohnEdwa commented 6 years ago

No, I mean rounding to nearest five. As in, the temperatures 110C, 120C, 130C, 140C are 230F, 248F, 266F, 284F. You can either round to nearest 10: 230, 250, 270, 280 which makes the steps either 10 or 20 and the error is between -4 and 4, or nearest 5: 230, 250, 265, 285 so the steps are either 15 or 20 and error is between -2 to 2.

fishingpro commented 6 years ago

No, I mean rounding to nearest five.

Yes, now I understand.

Since there will be a redesign of this place, it is possible to add Kelvin - that would be absolutely hardcore :)

fishingpro commented 6 years ago

Well, you know how a cosmologists uses a clock with Martian time, and a true-geeks must have a soldering iron with the Kelvin.

Kokokokoka commented 6 years ago

must have a soldering iron with the Kelvin.

That's actually a good idea.

ghost commented 6 years ago

I actually don't see any advantage with using another temperature scale since most of the users wouldn't probably feel confident with; its usage it's tipically scientific. Moreover it wouldn't solve the conversion problem because Celsius and Kelvin scales are pratically the same scale (±273.15).

I think that would compromise the usability of the iron whether you use metric system units in everyday life or not.

JohnEdwa commented 6 years ago

No, not as the only scale, of course, that's why my original suggestion had about as little seriousness as possible:

I vote for dropping both F and C and converting completely to Glorious Kelvin Master-Race :D

But if the system was converted from having separate C and F units and limits, and instead converted the displayed temperature at runtime, adding Kelvin would just be : (unit == K) ? print temp+273 : ( (unit == F) ? print CtoF(temp) : print temp); , and at that point why not add it :) ?

Ralim commented 6 years ago

Hi, Looks like I can sleep through quite a conversation.

Another thing to add to all of this is the iron pid and internals doesn't run on either temperature scale, and all readings are scaled back to C or F for UI only.

I will be adding a lockout to prevent the temp being set above 450C. As the internals cannot measure any temp above 455C,and the output goes open loop at that point.

I'm happy to add the conversions between the two and different scales for them as this is less size than adding lookup tables.

Rockman18 commented 6 years ago

@Ralim really sorry for not having any time at the moment to finish what I've started at Christmas. If someone want to implement that don't hesitate else I'll try to do it asap.

fishingpro commented 6 years ago

I'm happy to add the conversions between the two and different scales for them as this is less size than adding lookup tables.

In fact, in the table is not a fundamental necessity, I'm don't strong in the C language, but I believe that about such a function:

//Converting Celsius to Fahrenheit with rounding up to 10
int c2f(int c){
    int f = (c * 18 + 325) / 100;
    return floor(f) * 10;
}
fishingpro commented 6 years ago

The easiest and safest way is to always use Celcius internally and convert them to Fahrenheit when displayed (casting 'CELCIUS * 18 + 325)/10' to int works well)

If inside everything everything is read only in celsius, and the temperature parameter has a purely synthetic flag, then it will be enough in all places where the temperature display shows it through one simple function:

int ShowTemp(int c){
    int res = c;//Celsius
    if (systemSettings.temperatureInF)
        res = floor((c * 18 + 325) / 100)*10;//Fahrenheit
    if (systemSettings.temperatureInK)
        res = res + 270;//Kelvin - 270 more beautiful than 273.15
    return res;
}

you will get 3 beautiful scales Celsius 50..450 ° C Fahrenheit: 120..840 ° F Kelvin: 320..720 ° K

@Ralim really sorry for not having any time at the moment to finish what I've started at Christmas. If someone want to implement that don't hesitate else I'll try to do it asap.

I would gladly connect to your development, but I'm crooked/blunt and do not know how to build a build and therefore do not have the ability to locally check for changes.

Ralim commented 6 years ago

Happy to add Kelvin if people would actually use it :grin:

Internally everything is actually run using the ADC raw values, which are scaled into/out of the C or F scale at the time of use.

fishingpro commented 6 years ago

I hope that future generations will use only the metric system, and the temperature will be measured only in Kelvin. The concept of negative temperature in general is quite absurd.

Mrkvozrout commented 6 years ago

@fishingpro Ommiting 0.15 is the obvious choice (display width) but why to ommit also the 3? There is no point to ommit it. Base is 273 with increments of 10 for adjusting... correct and geeky

fishingpro commented 6 years ago

@Mrkvozrout I do not argue, maybe you are right, but I vote for 270;)

Kulturnilpferd commented 6 years ago

@fishingpro Just check the history there are several reasons why to use different temperature scales. Like this you can argue that we should only speak one language on earth because it would be simple xD

fishingpro commented 6 years ago

@Kulturnilpferd if you thought that I infringed on someone's right to measure in the usual format-it's not so :) I expressed the hope that someday there will be less factor separating people.

Kulturnilpferd commented 6 years ago

@fishingpro yep, that's right. Doesn't want to be rude :)

Ralim commented 6 years ago

Which step size would you like for the F scale?

Going to implement the conversions this weekend :)

2gnospam commented 6 years ago

I am not hard over on anything. I do like the F scale because I am used to it. I know what temps are good/bad for certain components. Tip life, etc. I guess I would like 5 deg F but 10 would be great also.

Cheers

JohnEdwa commented 6 years ago

Shame it's whole numbers only, it would have been so trollish to implement a system that shows fractions instead of decimals for the Fahrenheit scale. "Tip: 683 ⅝".

The Celcius scale uses 10C steps, which is 18F, so to me rounding up to 20F seems most logical.

fishingpro commented 6 years ago

@2gnospam What is worse than the above option, for a direct conversion of °F to °C with rounding to 10? Do you want rounding to within 5? I want to note that in any case the number of steps for any measurement system will remain the same as for °C 50..450 (discreteness 10). Those °F will be reduced to these values. For the limit in accordance with C 200, 210, 220, 230, 240, 250, the values ​​of 390, 410, 425, 445, 460, 480

@Ralim Apparently it's about this?

int ShowTemp(int c){
    int res = c;//Celsius
    if (systemSettings.temperatureInF)
        res = floor((c * 18 + 325) / 50)*5;//Fahrenheit (rounding 5)
    if (systemSettings.temperatureInK)
        res = res + 273;//Kelvin
    return res;
}
2gnospam commented 6 years ago

At any rate, I would be happy with 20 deg F steps, but would prefer something less. Again, I don't know exactly the best way to implement this, and you guys are the experts. Thanks

Ralim commented 6 years ago

I have implemented 20F steps since there is little point to having smaller step sizes for most use cases. I have also added rough conversion when you change between the two. I will need to polish this a bit more before release but is already a step in the right direction.

fishingpro commented 6 years ago

@Ralim assemble the build, we will test RC

Ralim commented 6 years ago

TS100_EN.hex.zip Hi @fishingpro, @2gnospam, @JohnEdwa , @Rockman18 , Above is the latest test build, let me know how this goes for you. I havent got my iron handy to test on for a bit, so be careful :)

fishingpro commented 6 years ago

@Ralim, Something went wrong...:)

  1. Set the temperature to 300 C
  2. We change the unit of measure F
  3. Return the unit C
  4. Temperature set to 290
  5. etc. you can switch several times in both directions and reduce it by more than 10 degrees.
  1. Enable detailed information in standby mode
  2. Heat the soldering iron to 300 C
  3. Turn off soldering iron
  4. Turn on the soldering iron
  5. The tempature will gradually rise from 0 to the current value.

PS: This solution is not idyllic, but it's better than now, I'd like to avoid the problem # 2 PS2: Maybe Problem # 3 is not a bug, but a feature, but before that it was better :)

Ralim commented 6 years ago

I'm not surprised, thank you for testing :smile:

Problem 1 is because of using a floor operation, just didnt want to add a rounding function. Problem 2, if you could find a trigger it would be great. This change should not have affected the menu's operation.

Problem 3 is from increasing the filtering on the displayed values to make the screen display more readable.

fishingpro commented 6 years ago

Problem 2, if you could find a trigger it would be great. This change should not have affected the menu's operation.

@Ralim the problem is not reproduced, perhaps you added new settings but did not initialize their initial value and at the first tests the software fell, with the setting of some value and the problem became no longer relevant.

Ralim commented 6 years ago

@fishingpro I have not been able to reproduce your issue either at the moment.

Going to push this up as an alpha and see what people think.