Intentional use of negative value to represent max unsigned

[safocl]

This code depends on the integer rank and returns true

C_ESR_METER/C_ESR_METER.ino

Line 1206 in e8d24cf

if (AutoOffTimer == -1){

example:
https://godbolt.org/z/n7Tavjc6W

relevant rules:
https://eel.is/c++draft/expr.arith.conv#1.5
https://eel.is/c++draft/expr.eq#8

but this example always false:
https://godbolt.org/z/a6jhjae1E

The int type has a non-fixed size in bits (implementation dependent) (-1 is a literal of type int)

If the stdint.h header is available, the UINTN_MAX definition can be used (N is the bit size of the integer type).

[latchdevel]

Thanks for raising this.

The variable in question is declared as uint32_t, therefore, there’s no ambiguity regarding its rank or capacity.

C_ESR_METER/C_ESR_METER.ino

Line 201 in e8d24cf

uint32_t AutoOffTimer = 0; // Millis to auto-off (0 to disable)

When the variable is assigned the value -1, it's intentional and documented in the comment.

C_ESR_METER/C_ESR_METER.ino

Lines 1188 to 1189 in e8d24cf

	// Force max unsigned long to enable manual power-off
	AutoOffTimer = -1;

The warning during comparison is intentionally suppressed, which shows full awareness of the signed/unsigned behavior and confirms it's a deliberate design choice.

C_ESR_METER/C_ESR_METER.ino

Lines 1204 to 1207 in e8d24cf

	#pragma GCC diagnostic push
	#pragma GCC diagnostic ignored "-Wsign-compare"
	if (AutoOffTimer == -1){
	#pragma GCC diagnostic pop

That said, it's worth noting that UINT32_MAX is available via stdint.h and could have been used instead of -1 for clarity. This might improve readability and make the intent even more explicit.

#define INT32_MAX 0x7fffffffLL
#define INT32_MIN (-INT32_MAX - 1LL)
#define UINT32_MAX (__CONCAT(INT32_MAX, U) * 2ULL + 1ULL)

https://github.com/avrdudes/avr-libc/blob/3eed8ade37607c8e617eac1cb13f82cb9cfa4409/include/stdint.h#L372-L374

Good point — I’ll probably switch to UINT32_MAX to make it more explicit.

Thanks you so much.
Best regards.

[safocl]

The variable in question is declared as uint32_t, therefore, there’s no ambiguity regarding its rank or capacity.

yes, but int has no fixed size and can be at least 16 bits.
https://eel.is/c++draft/basic.fundamental#tab:basic.fundamental.width
if wins this rule:

(1.5.3.2) -- Otherwise, if S can represent all of the values of U, C is S.

then val > 0 will be compared to -1 (both signed) -- which is always false and is not expected.

[latchdevel]

Okay, understood. Thanks for the clarification.

The code behaves correctly because the -1 literals are interpreted as 32-bit int.
A hypothetical issue would only arise if the variable were 16-bit and the compiler promoted it to a 32-bit int during the comparison.

This could be improved by explicitly specifying the type of the -1 literal during the comparison:

if ( AutoOffTimer == static_cast<uint32_t>(-1) ) {

While there's no issue with the current implementation, I think it could be slightly improved by using UINT32_MAX, which would be more correct and help avoid potential confusion.