ATMega328 / Arduino

[Vik]

I'll add a cast to uint32_t to both the parameter and CH_FREQUENCY in next version.

Giovanni

Hi Giovanni,

The AVR patches are in trunk now, but they don't really work because of this bug. Can we fix it in trunk?

Btw, if you are going to use uint32_t, then you might want to change 1L and 100000L to UL to prevent further casting.

Cheers,
Vik

[Giovanni]

Hi, I committed a fix, please verify.

Giovanni

[utzig]

Works now, thanks!

[Vik]

Thanks for this, I'll test this evening but I expect that it will work fine.

A pedantic point about S2ST() though:

Code: Select all

 #define S2ST(sec)                                                           \
-  ((systime_t)((sec) * CH_FREQUENCY))
+  ((systime_t)(((uint32_t)(sec)) * ((uint32_t)CH_FREQUENCY)))

This is probably a waste on architectures where systime_t is uint16_t (mostly the 8 bit controllers).

If this is the case, then the 32 bit result will immediately get truncated to 16 bit, so the computation is more expensive but it doesn't accomplish anything. How about casting to systime_t in this case, like this?

Code: Select all

((systime_t)(((systime_t)(sec)) * ((systime_t)CH_FREQUENCY)))

Cheers,
Vik

[Giovanni]

I would verify first, being both operands constants (almost always) and then casted to systime_t there would be no 32bits operations involved at runtime, the same is true for other macros.

Anyway, I would rather leave those non-casted rather than casted to systime_t since in that case there is no division.

Giovanni