[riot-notifications] [RIOT-OS/RIOT] test/xtimer_drift: DEBUG_PINS (#10173)

Peter Kietzmann notifications at github.com
Tue May 21 14:28:03 CEST 2019


PeterKietzmann commented on this pull request.

@Josar I ran the application on a samr21-xpro with 
```
CFLAGS += -DSLACKER_THREAD_PIN=GPIO_PIN\(0,22\)
CFLAGS += -DMAIN_THREAD_PIN=GPIO_PIN\(1,3\)
CFLAGS += -DPRINT_THREAD_PIN=GPIO_PIN\(1,22\)
```
Results look slightly different in comparison to yours, but they seem reasonable to me.

![xtimer_drift_samr21_dbgpins](https://user-images.githubusercontent.com/7765855/58095614-b6668a00-7bd3-11e9-92fc-d205a510a34a.png)

In a nutshell:
- print occurs every second
- main occurs every ~15.6ms and takes a little longer when print happens around the same time
- slacker occurs in (non constant) intervals between 1...10ms


> @@ -5,4 +5,19 @@ BOARD_INSUFFICIENT_MEMORY := arduino-duemilanove arduino-uno \
 
 USEMODULE += xtimer
 
+# Port and pin configuration for probing with oscilloscope
+# Define Test pin for hardware timer interrupt, hardware dependent
+# ATmega
+#CFLAGS += -DDEBUG_TIMER_PORT=PORTF
+#CFLAGS += -DDEBUG_TIMER_DDR=DDRF
+#CFLAGS += -DDEBUG_TIMER_PIN=PORTF4
+

This test should be independent of the atmega implementation. Ideally, we would have a generic debug pin for this purpose configured for every platform. Now that this is out of scope for your PR, I'd say we remove it.

> @@ -5,4 +5,19 @@ BOARD_INSUFFICIENT_MEMORY := arduino-duemilanove arduino-uno \
 
 USEMODULE += xtimer
 
+# Port and pin configuration for probing with oscilloscope
+# Define Test pin for hardware timer interrupt, hardware dependent
+# ATmega
+#CFLAGS += -DDEBUG_TIMER_PORT=PORTF
+#CFLAGS += -DDEBUG_TIMER_DDR=DDRF
+#CFLAGS += -DDEBUG_TIMER_PIN=PORTF4
+
+# Define test probing pins GPIO API based.
+# Port number should be found in port enum e.g in cpu/include/periph_cpu.h
+#FEATURES_REQUIRED += periph_gpio

Is there a single platform that has no GPIO driver but xtimer support? In my opinion it should be fine to always include that feature (otherwise we would see what Murdoch says later)

> @@ -5,4 +5,19 @@ BOARD_INSUFFICIENT_MEMORY := arduino-duemilanove arduino-uno \
 
 USEMODULE += xtimer
 
+# Port and pin configuration for probing with oscilloscope
+# Define Test pin for hardware timer interrupt, hardware dependent
+# ATmega
+#CFLAGS += -DDEBUG_TIMER_PORT=PORTF
+#CFLAGS += -DDEBUG_TIMER_DDR=DDRF
+#CFLAGS += -DDEBUG_TIMER_PIN=PORTF4
+
+# Define test probing pins GPIO API based.
+# Port number should be found in port enum e.g in cpu/include/periph_cpu.h
+#FEATURES_REQUIRED += periph_gpio
+# Jiminy probing Pins

If you really wanna have the jiminy config here, please define the CFLAGS in 
```
ifneq (,$(filter jiminy,$(BOARD)))

endif
```

> @@ -67,6 +72,11 @@ void *slacker_thread(void *arg)
     (void) arg;
     timex_t now;
 
+#if defined(SLACKER_THREAD_PIN)
+    gpio_t slacker_pin = SLACKER_THREAD_PIN;
+    gpio_init(slacker_pin, GPIO_OUT);

Why not directly `gpio_init(SLACKER_THREAD_PIN, GPIO_OUT);` here and following?

> @@ -79,7 +89,10 @@ void *slacker_thread(void *arg)
         struct timer_msg *tmsg = m.content.ptr;
         xtimer_now_timex(&now);
         xtimer_usleep(TEST_MSG_RX_USLEEP);
-
+#if defined(SLACKER_THREAD_PIN)
+        gpio_set(slacker_pin);
+        gpio_clear(slacker_pin);

Probably a matter of taste, but wouldn't a single toggle be simpler and easier to read on the scope?

-- 
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/10173#pullrequestreview-240001946
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.riot-os.org/pipermail/notifications/attachments/20190521/4a27d03c/attachment.html>


More information about the notifications mailing list