zephyrCommon: Improved tone implemetation#133
zephyrCommon: Improved tone implemetation#133DhruvaG2000 merged 1 commit intozephyrproject-rtos:nextfrom
tone implemetation#133Conversation
cores/arduino/zephyrCommon.cpp
Outdated
| gpio_pin_set_dt(spec, 0); | ||
| } else { | ||
| gpio_pin_toggle_dt(spec); | ||
| if (pt->count != UINT32_MAX) { |
There was a problem hiding this comment.
Maybe it would be better to add a bool infinite to struct pin_timer? Other than that, it looks good to me.
Using only a single timer is great, should help fix a lot of edge cases.
There was a problem hiding this comment.
Maybe it would be better to add a bool infinite to struct pin_timer?
I applied your suggestion.
soburi
force-pushed
the
improve_tone
branch
2 times, most recently
from
b78e5b1 to
4534934
Compare
cores/arduino/zephyrCommon.cpp
Outdated
| struct k_timer timer; | ||
| pin_size_t pin; | ||
| bool infinity; | ||
| uint32_t count; |
There was a problem hiding this comment.
I think it would be better to place bool infinity after count. Decreasing order of size for efficient size: https://markaicode.com/ultimate-guide-to-c-structure-memory-alignment-and-padding/
struct pin_timer {
struct k_timer timer;
uint32_t count;
pin_size_t pin;
bool infinity;
} arduino_pin_timers[MAX_TONE_PINS];Also moved pin down since it can either be 32bit (=count) or 8 bit (<count).
DhruvaG2000
left a comment
DhruvaG2000
left a comment
There was a problem hiding this comment.
Also $subject should be imperative -> zephyrCommon: Improve tone implemetation
cores/arduino/zephyrCommon.cpp
Outdated
| } | ||
|
|
||
| arduino_pin_timers[pinNumber].infinity = (duration == 0); | ||
| arduino_pin_timers[pinNumber].count = ((uint64_t)duration * frequency) / 500ULL; |
There was a problem hiding this comment.
Please avoid usage of magic numbers, can you call out why this is / 500?
There was a problem hiding this comment.
Something like this helps make it more readable
#define MS_PER_SECOND 1000ULL
#define TOGGLES_PER_CYCLE 2ULL
#define TONE_COUNT_DIVISOR (MS_PER_SECOND / TOGGLES_PER_CYCLE) // = 500
…log-level Removed llext log level setting in variant config file
soburi
force-pushed
the
improve_tone
branch
from
1565d9b to
9071c2c
Compare
Improved to align more closely with the standard Arduino implementation. - Eliminated the end timer and used counting to determine the end. - Improved to handle infinity correctly. - Added a guard to prevent the timeout value from reaching 0. - Set the GPIO value to 0 when the timer starts. Signed-off-by: TOKITA Hiroshi <tokita.hiroshi@gmail.com>
soburi
force-pushed
the
improve_tone
branch
from
9071c2c to
50ce83d
Compare
DhruvaG2000
left a comment
DhruvaG2000
left a comment
There was a problem hiding this comment.
Sigh... all the CI failures seem to be github/ infra related :(
Ran checkpatch locally and saw no errors. So approving.
@soburi assuming you've had a chance to build test locally?
Yes, I verified build and run with my arduino_mkrzero. |
3 participants
Improved to align more closely with the standard Arduino implementation.