Add light versions. #32
Conversation
Light versions of Thread and StaticThreadController that uses less memory.
There was a problem hiding this comment.
Overall it can be good lightweight standalone solution, but doesn't seem it is compatible with current library. Basically you can use either one kind of threads or another but can't mix it together. Both normal and lite threads have some significant differences which may affect application design (I mean enabling\disabling functionality and inheritance).
| #endif | ||
|
|
||
| #define TIME_INT uint16_t | ||
| #define TIME_OVERFLOW 0x8000 // uint16_t = 0x8000 or uint32_t = 0x80000000 |
There was a problem hiding this comment.
I'm just curious why do you define a new type in that way? Do you need extra flexibility to easily change underlying type? Don't you consider to make a template class in this case?
Anyway, why do you use define instead of typedef or using?
Also overflow value can be defined based on type information, something like this:
using TIME_INT=uint16_t;
constexpr TIME_INT TIME_OVERFLOW = (1 << (sizeof (TIME_INT) * 8 - 1));Seems it should be done in the mainline too (instead of using 0x80000000)...
There was a problem hiding this comment.
Thank you for giving this correction. I used bad practice before.
| // If the "sign" bit is set the signed difference would be negative | ||
| bool time_remaining = (time - _cached_next_run) & TIME_OVERFLOW; | ||
|
|
||
| // Exceeded the time limit, AND is enabled? Then should run... |
There was a problem hiding this comment.
// Exceeded the time limit? Then should run...
ThreadLite is always enabled :)
| ThreadLite::ThreadLite(void (*callback)(void), TIME_INT _interval) { | ||
| _onRun = callback; | ||
| _cached_next_run = 0; | ||
| last_run = 0; |
There was a problem hiding this comment.
Avoiding to initialize last_run may lead to missed first interval.
For example, if we set very small interval, for example 10ms and current millis() is greater than 32780 (we still use unsigned type, so it can be) then _cached_next_run will be 10 (lasr_run + interval) and sign bit will be set for all the operations until we overflow and thus should_run() will return false.
So for the first run we have to wait around 32 seconds instead of 10ms.
It's kind of rare situation, but I think last_run should be initialized to millis() here.
There was a problem hiding this comment.
You're right. I had a special case in my project and could not use initialization to save memory.
|
The lite version is really incompatible with the original library. This is an alternative. It would be better to create a new lite library instead of the merger? And how is it better to do this? I do not have the experience of pulling a request, but I think that this version can help someone. |
|
I'm not the maintainer of this project, so I can't make a decision. But I guess the best solution is to make separate library (with proper documentation and examples) and put some cross-reference to it from this project to help people to find lightweight version. |
|
Hi @stanmain! Thanks for reaching out and making contribution to the library! We need to understand how compatible this can be to choose if it will be merged into ArduinoThread, or a new Library (we could reference it in this case in the README). I have a few questions to guide us during this process.
@CAHEK7 thank you very much for the help! |
Light versions of Thread and StaticThreadController that uses less memory.
Maximum period is only 32,767 seconds.
It is recommended to use only in case of a memory deficit.