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

Calibrate CJC at next boot #1398

Merged
merged 95 commits into from
Sep 10, 2022
Merged

Calibrate CJC at next boot #1398

merged 95 commits into from
Sep 10, 2022

Conversation

discip
Copy link
Collaborator

@discip discip commented Aug 29, 2022

After reading this #1387 post, I had the idea to make CJC (Cold Junction Compensation) calibration as accurate as possible by executing it at boot.

This is an attempt to implement that to resolve #1387.

Tested this on both TS80P & Pinecil.

What has changed:

  1. Calibrate temperature is now called Calibrate CJC at next boot.
    1. It is now performed, as the name implies, exclusively at the next boot.
    2. The calibration will only start if the tip is installed & the temp difference between tip & handle is < 10°C !
    3. This setting gets deactivated every time it is executed. (No accidental calibration!)
  2. Debug Menu was partially rearranged and the entries were aligned, some of which were renamed to unify the appearance.
    1. There is now one more entry named Tip O, which indicates the actual CJC offset.
    2. CHan now Han C, is displayed directly in °C to match Tip C & Max C.
  3. All (at least I hope so 😅) the corresponding documentations have been updated.

@discip
Copy link
Collaborator Author

discip commented Sep 3, 2022

@Ralim
Adding spaces:
discip@9c15795

Makes the TS80P build fail:
https://github.com/discip/IronOS/runs/8166069844?check_suite_focus=true

Do you have any clue on why only the TS80P is affected?

@Ralim
Copy link
Owner

Ralim commented Sep 3, 2022

Out of space, those extra bytes pushed you over the space left in flash

@discip
Copy link
Collaborator Author

discip commented Sep 3, 2022

btw: Do you have any idea what could help us to reclaim some space again?

@discip
Copy link
Collaborator Author

discip commented Sep 3, 2022

@Ralim
This can be merged now. 😊

@Ralim
Copy link
Owner

Ralim commented Sep 3, 2022

btw: Do you have any idea what could help us to reclaim some space again?

Most likely future features will be masked off from the Miniware Irons (TS100/TS80/TS80P) at this point
There are things I would like to do, but they are small changes, there isnt much that would free up a significant chunk.

I want to look into compressing all languages (rather than just multi-lang packs) but not sure if that net gains any space or not.

Copy link
Owner

@Ralim Ralim left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good to me.
I'll hold off merging until I get a sec to test

@discip
Copy link
Collaborator Author

discip commented Sep 3, 2022

I am good with that! 😃

@discip
Copy link
Collaborator Author

discip commented Sep 3, 2022

@Ralim
I had to update the Debug Menu once more to give the alignment a chance.
Text is aligned left and numbers so that the integers are on par.

Hope you don't mind.

@discip discip requested a review from Ralim September 7, 2022 18:24
@discip
Copy link
Collaborator Author

discip commented Sep 7, 2022

@Ralim
Just a reminder:
Please don't forget to check if this also works for the TS100 & the Pinecil v2 before merging.

Thank you

@discip
Copy link
Collaborator Author

discip commented Sep 9, 2022

  1. To save space I thought about removing unused features and found that I had never even tried the locking mode!
    Do you know if this is a crucial feature or if someone would miss it if it was removed?

    In one of my branches of my fork I did exactly that and I got ~80 KB back.

  2. And by removing the invert screen option in addition to the previous one, we save another ~30 KB.

So creating a PR would not be an issue. 😊

  1. I want to look into compressing all languages (rather than just multi-lang packs) but not sure if that net gains any space or not.

    1. Is there anything I could help you with on that?
    2. Or do you think that involves to much in-depth coding?
    3. Maybe you could point me into the right direction?

@Ralim
Copy link
Owner

Ralim commented Sep 10, 2022

I do not want to remove locking mode as it is not a large amount of space, not sure about the 80KB figure?
It is also a reasonably new contribution hmm

Liekwise invert screen is liked by some so prefer not to take out the option, but might need to figure out about reducing its size cost?

@Ralim
Copy link
Owner

Ralim commented Sep 10, 2022

For the language option, I would need to figure out how the current system really works (I didnt write it)
IT may be as simple as removing the non-compressed path in the makefile but reallllllllly not sure off the top of my head

@discip
Copy link
Collaborator Author

discip commented Sep 10, 2022

  1. I do not want to remove locking mode ...

    I'm fine with that. Just wanted to throw in my 2 cents. 😅

    ... not sure about the 80KB figure

    Sorry, my bad! I used the size of the whole zip file as reference! 🤦‍♂️
    So, for eg: TS80P_EN.dfu it saves about 511 bytes or 687 bytes respectively.

  2. ... IT may be as simple as removing the non-compressed path in the makefile ...

    Ok, then I guess I just have to wait until you have time, because that alone is beyond my understanding! 🤣

  3. Have you had a chance to look into the changes so far?

@Ralim
Copy link
Owner

Ralim commented Sep 10, 2022

Tested on TS100 and works perfectly, so I think its good to gooooooo

@Ralim Ralim merged commit c89db78 into Ralim:dev Sep 10, 2022
@discip discip deleted the calibrate_tip_CJC branch September 10, 2022 10:58
@discip
Copy link
Collaborator Author

discip commented Sep 10, 2022

@Ralim
I guess the 'bug-tag' can now be removed from #1387 (comment).

Ralim added a commit that referenced this pull request Oct 24, 2022
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.

Calibrate Temperature
2 participants