Keep the locked warning for some time

[neon12345]

This is an untested draft to keep the locked warning for some time, independet of how long a button is pressed.

[neon12345]

Does not work because handleSolderingButtons is not called when nothing happens with the buttons?!

[neon12345]

Timer logic was wrong, it seems to work now with 1s display time. Can be shorter on tick overflow.

[discip]

@neon12345

1. First of all, an explanation:
   Unfortunately I wasn't allowed to add the changes I committed as a suggestion like the one above, so I hope you like it, otherwise please change it back or if you insist I will do it myself. 😊

2. I tested this PR on the TS80P and noticed that sometimes for what ever reason the !LOCKED! warning appeared only briefly, whereas the other time it stayed until the sleeping mode kicked in.

   @Ralim
   Do you maybe have an idea?

[neon12345]

The implementation uses the remaining 14 bits of the state as a tick counter. So with e.g. 1000 ticks per second and one-second display time, there is a 1 out of 16 seconds window for a short display to happen. Increasing the state to 32-bit can make this less likely.

Looking at other uses of the global tick counter, it seems the current policy is for the device not to be powered until an overflow can happen.

[neon12345]

@discip

If it is ok to block the task, the tick stuff can be removed.

[Ralim]

Rather than bit packing this I would advise to use a uint32_t or a dedicated uint16_t so that rollover/under wraps and does roughly what is expected. You can use one of the other state variables for the time by itself. If none a spare, add another one :)

I do agree with the concept though, this is good as it keeps it non-blocking which is the end-game goal of the UI code.

[Ralim]

Hia @neon12345 ,
Apologies for delays in getting to this.

The implementation uses the remaining 14 bits of the state as a tick counter. So with e.g. 1000 ticks per second and one-second display time, there is a 1 out of 16 seconds window for a short display to happen. Increasing the state to 32-bit can make this less likely.

Yeah, I would strongly suggest using a 32bit state variable if you can. I added the comment in the code review to keep it a bit more trackable. I think your on the right path though.

Looking at other uses of the global tick counter, it seems the current policy is for the device not to be powered until an overflow can happen.

The general logic is to use the TickType_t (aka uint32_t) and to use the rollover/under of the uint32_t in the maths. Which often will work. But also its not expected for a device to be powered for 49 days straight often.

[neon12345]

I added state7, but state4 is possibly also safe to use.

[discip]

@neon12345
This works better now, but what I don't like is the fact that the LOCKED / UNLOCKED info is not displayed long enough.

Please test this build: https://github.com/discip/IronOS/actions/runs/10274229163
It works the way I imagine it would, without causing confusion if you release the keys too early.

I've tested it successfully on all of these: TS80P, S60P & Pinecil-2

@Ralim
Could you please also give it a go and let me know if this is reasonable? 😊

[neon12345]

@discip

Could you explain the changes compared to this pull request? Since I would only release the buttons after the confirmation, I am fine with the double-button handling.

[discip]

@neon12345
I'm inclined to let go of the buttons as soon as a change on the OLED takes place.
The way you made it work via this PR is to switch to the soldering interface as soon as you let the buttons go.

The little change I made does only let the mentioned confirmation texts stay a bit longer. 😊

Also this line:

IronOS/source/Core/Threads/UI/logic/Soldering.cpp

Line 29 in dfbf45a

warnUser(translatedString(Tr->WarningKeysLockedString), buttons);

will not be missed if it gets removed.

The warning will be still displayed.

[neon12345]

@discip

If you let go of the buttons, you will then have to wait one second for the message to go away. Why not just press (as long as you want) until you recognize what happened? 😃

But if this is what you want, I would suggest using the timer ticks and states instead of vTaskDelay.

[discip]

@neon12345

1. But if this is what you want, I would suggest using the timer ticks and states instead of vTaskDelay.

   I'm not able to pull this off, maybe you could do that for me. 😊

2. Also there is a total of 3 messages regarding the locking function but with your implementation only 2 of them are used.

   IronOS/Translations/translation_EN.json

   Lines 20 to 28 in 2bce5b7

   	},
   	"LockingKeysString": {
   	"message": "LOCKED"
   	},
   	"UnlockingKeysString": {
   	"message": "UNLOCKED"
   	},
   	"WarningKeysLockedString": {
   	"message": "!LOCKED!"

   So either we remove the LOCKED message entirely and only use !LOCKED! instead, or we make (LOCKED) appear directly after locking the buttons and as long as the buttons are locked every press of the buttons will display !LOCKED!.

[neon12345]

@discip

I have changed the message used for state 2 to LockingKeysString, to make it more consistent. For 1: This would be multiple times the effort of the current implementation. I can not commit more time for this at the moment.

[discip]

That'll do! 👍

Thank you! 😊