[riot-notifications] [RIOT-OS/RIOT] periph/timer: add timer_set_periodic() (#13902)

Marian Buschsieweke notifications at github.com
Thu May 28 09:19:03 CEST 2020


@maribu commented on this pull request.

Two more comments inline.

<details><summary>I tested successfully on the Arduino Mega2560 ...</summary>

```
make BOARD=arduino-mega2560 -C tests/periph_timer_periodic/ flash test
[...]
Help: Press s to start test, r to print it is ready
READY
s
START
main(): This is RIOT! (Version: 2020.07-devel-812-g8bf8d-ben)

Running Timer 0 at 62500 Hz.
One counter cycle is 15625 ticks or 250 ms
Will print 'tick' every second / every 4 cycles.

TEST START
[0] tick
[0] tick
[0] tick
[0] tick
[0] tick
[0] tick

Cycles:
channel 0 = 22	[OK]
channel 1 = 00	[OK]
channel 2 = 00	[OK]
TEST SUCCEEDED

make: Leaving directory '/home/maribu/Repos/software/RIOT/tests/periph_timer_periodic'
```
</details>

<details><summary>... and on the MSB-A2.</summary>

```
make BOARD=msba2 -C tests/periph_timer_periodic/ flash test
[...]
Help: Press s to start test, r to print it is ready
READY
s
START
main(): This is RIOT! (Version: 2020.07-devel-812-g8bf8d-ben)

Running Timer 0 at 62500 Hz.
One counter cycle is 15625 ticks or 250 ms
Will print 'tick' every second / every 4 cycles.

TEST START
[0] tick
[0] tick
[0] tick
[0] tick
[0] tick
[0] tick

Cycles:
channel 0 = 22	[OK]
channel 1 = 00	[OK]
channel 2 = 00	[OK]
channel 3 = 00	[OK]
TEST SUCCEEDED

make: Leaving directory '/home/maribu/Repos/software/RIOT/tests/periph_timer_periodic'
```
</details>

<details><summary>I also verified that the test is failing as expected, when the timer is not periodically running</summary>

##### Breaking the periodic implementation:

``` diff
diff --git a/cpu/lpc2387/periph/timer.c b/cpu/lpc2387/periph/timer.c
index c4a1f8fb3d..7658532b87 100644
--- a/cpu/lpc2387/periph/timer.c
+++ b/cpu/lpc2387/periph/timer.c
@@ -233,7 +233,7 @@ static inline void chan_handler(lpc23xx_timer_t *dev, unsigned tim_num, unsigned
     }
 
     dev->IR |= mask;
-    if (is_oneshot(tim_num, chan_num)) {
+    if (1 || is_oneshot(tim_num, chan_num)) {
         dev->MCR &= ~(1 << (chan_num * 3));
     }
 
```

##### Re-ran the test with broken code:
```
make BOARD=msba2 -C tests/periph_timer_periodic/ flash test
[...]
Help: Press s to start test, r to print it is ready
READY
s
START
main(): This is RIOT! (Version: 2020.07-devel-812-g8bf8d-ben)

Running Timer 0 at 62500 Hz.
One counter cycle is 15625 ticks or 250 ms
Will print 'tick' every second / every 4 cycles.

TEST START
[0] tick
Timeout in expect script at "child.expect_exact('TEST SUCCEEDED')" (tests/periph_timer_periodic/tests/01-run.py:17)

make: *** [/home/maribu/Repos/software/RIOT/tests/periph_timer_periodic/../../Makefile.include:769: test] Error 1
make: Leaving directory '/home/maribu/Repos/software/RIOT/tests/periph_timer_periodic'


```

</details>

> +    switch (channel) {
+    case 0:
+        dev(tim)->INTFLAG.reg = TC_INTFLAG_MC0;
+
+        /* only CC0 can be use to set TOP */
+        if (flags & TIM_FLAG_RESET_ON_MATCH) {
+            _set_mfrq(tim);
+        } else {
+            _set_nfrq(tim);
+        }
+
+        _set_cc(tim, 0, value);
+        dev(tim)->INTENSET.reg = TC_INTENSET_MC0;
+        break;
+    case 1:
+        assert((flags & TIM_FLAG_RESET_ON_MATCH) == 0);

```suggestion
        if (flags & TIM_FLAG_RESET_ON_MATCH) {
            assert(0);
            return -1;
        }
```

With the API allowing to return an error, I would prefer to fall back to this with `NDEBUG`.

> + * @ingroup     tests
+ * @{
+ *
+ * @file
+ * @brief       Periodic timer test application
+ *
+ * @author      Benjamin Valentin <benpicco at beuth-hochschule.de>
+ *
+ * @}
+ */
+
+#include <stdio.h>
+#include <stdint.h>
+
+#include "board.h"
+#include "expect.h"

```suggestion
#include "test_utils/expect.h"
```

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/RIOT-OS/RIOT/pull/13902#pullrequestreview-419824664
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.riot-os.org/pipermail/notifications/attachments/20200528/5c2c19ff/attachment-0001.htm>


More information about the notifications mailing list