boards: add initial support for the PINE64 PineTime smartwatch by kaspar030 · Pull Request #12552 · RIOT-OS/RIOT

kaspar030 · 2019-10-23T11:56:05Z

Contribution description

I just got the PineTime devkit. It was a breeze to port, from opening the device to hello world took <15minutes. Thanks @aabadie @haukepetersen et al for the nice abstractions there!

For more information about the board, please look here:
https://wiki.pine64.org/index.php/PineTime

edit
This currently only adds the basic board configuration and flashing etc. No drivers are used, yet.

Testing procedure

AFAIK they just started shipping the devkits, so I'm probably the only maintainer who has one.
I'll provide output and test results.

For now, maybe concentrate on the integration.

Issues/PRs references

kaspar030 · 2019-10-23T11:56:51Z

examples/hello-world output:

[kaspar@ng riot/examples/hello-world (add_pinetime_support)]$ BOARD=pinetime make term
/home/kaspar/src/riot/dist/tools/jlink/jlink.sh term_rtt
### Starting RTT terminal ###
Twisted not available, please install it if you want to use pyterm's JSON capabilities
2019-10-23 13:52:20,988 # Host name for TCP connection is missing, defaulting to "localhost"
2019-10-23 13:52:20,988 # Connect to localhost:19021
Welcome to pyterm!
Type '/exit' to exit.
2019-10-23 13:52:21,993 # SEGGER J-Link V6.44e - Real time terminal output
2019-10-23 13:52:21,993 # SEGGER J-Link EDU V10.1, SN=260107551
2019-10-23 13:52:21,994 # Process: JLinkExe
2019-10-23 13:52:21,994 # main(): This is RIOT! (Version: 2020.01-devel-284-gdb33e-add_pinetime_support)
2019-10-23 13:52:21,994 # Hello World!
2019-10-23 13:52:21,995 # You are running RIOT on a(n) pinetime board.
2019-10-23 13:52:21,995 # This board features a(n) nrf52 MCU.

Other than that, I tested xtimer_msg, which works fine.

cgundogan · 2019-10-23T12:07:37Z

dist/tools/codespell/check.sh

 CODESPELL_OPTS+=" --check-hidden"
 # Disable false positives "nd  => and, 2nd", "WAN => WANT", "od => of"
-CODESPELL_OPTS+=" --ignore-words-list=ND,nd,WAN,od"
+CODESPELL_OPTS+=" --ignore-words-list ND,nd,wan,od,dout"


Huh? Was this supposed to end up in this PR?

no, sorry, thanks!

kaspar030 · 2019-10-23T12:17:07Z

force-pushed removal of the unrelated commit

bergzand · 2019-10-23T12:46:30Z

Awesome!

Skimming through the display controller datasheet, it looks like it is compatible with the display driver from #9948

kaspar030 · 2019-10-23T13:06:01Z

Skimming through the display controller datasheet, it looks like it is compatible with the display driver from #9948

Yeah! I've already configured the settings. 1 hour of RIOT hello-world drained the battery, need to recharge before I can try...

bergzand · 2019-10-23T13:07:38Z

Yeah! I've already configured the settings. 1 hour of RIOT hello-world drained the battery, need to recharge before I can try...

Can we have some proof when you have the display working? 😄

kaspar030 · 2019-10-23T14:17:36Z

Can we have some proof when you have the display working? smile

Sure! 😄 It's all black at the moment, though.

patchedsoul · 2019-12-27T13:00:38Z

@kaspar030 @bergzand Will this be landing in the January release by chance?

kaspar030 · 2020-01-09T14:59:05Z

@kaspar030 @bergzand Will this be landing in the January release by chance?

I'll put this to high-priority. With the ili9341 merged, this should be possible. ping @fjmolinas, this would be awesome to have!

bergzand · 2020-01-09T15:00:20Z

I'll put this to high-priority. With the ili9341 merged, this should be possible. ping @fjmolinas, this would be awesome to have!

I should receive my PineTime in the next few days, I can test this PR as soon as I have it.

kaspar030 · 2020-01-09T15:10:57Z

I've got some more code here: https://github.com/kaspar030/pinetime-riot

The branch includes a simple module for stdio on the display. It is based on a pre-merged ili9341, though.

bergzand

Initial review, nothing tested so far

bergzand · 2020-01-09T15:01:11Z

boards/pinetime/Makefile.include

+# for this board, we are using Segger's RTT as default terminal interface
+USEMODULE += stdio_rtt
+TERMPROG = $(RIOTTOOLS)/jlink/jlink.sh
+TERMFLAGS = term_rtt


Suggested change

TERMFLAGS = term_rtt

TERMFLAGS = term-rtt

This is coppied, others use term_rtt:

[kaspar@ng riot (add_pinetime_support)]$ gg -i term.rtt Makefile.include:# TERMFLAGS must be exported for `jlink.sh term_rtt`. boards/hamilton/Makefile.include:TERMFLAGS = term_rtt boards/pinetime/Makefile.include:TERMFLAGS = term_rtt boards/ruuvitag/Makefile.include:TERMFLAGS = term_rtt boards/thingy52/Makefile.include:TERMFLAGS = term_rtt dist/tools/jlink/jlink.sh: term_rtt)

And it was modified in #12733 to term-rtt.

Thanks. fixed

bergzand · 2020-01-09T15:03:31Z

boards/pinetime/include/gpio_params.h

+extern "C" {
+#endif
+
+/* move on, nothing to see here */


Can you add the GPIO button available on the PineTime here?

bergzand · 2020-01-09T15:04:52Z

boards/pinetime/include/gpio_params.h

+extern "C" {
+#endif
+
+/* move on, nothing to see here */


And I guess this should be extended with the vibrator GPIO as soon as SAUL supports vibrators. This can of course wait for a follow up :)

yup, its there. the vibrator is just a GPIO. On my devkit it worked only when on the cradle, though.
Anyhow, the define should be there now.

bergzand · 2020-01-09T15:05:34Z

boards/pinetime/include/periph_conf.h

+#include "cfg_rtt_default.h"
+#include "cfg_spi_default.h"
+#include "cfg_timer_default.h"
+


Should the battery ADC line be included here?

~~folo :)~~ done

bergzand · 2020-01-09T15:06:09Z

boards/pinetime/Makefile.features

+
+# Put defined MCU peripherals here (in alphabetical order)
+#FEATURES_PROVIDED += periph_i2c
+#FEATURES_PROVIDED += periph_uart


UART is not available on the board right?

Isn't there a debug pin or two that could be repurposed? but yeah, not really.

bergzand · 2020-01-09T15:08:17Z

boards/pinetime/Makefile

@@ -0,0 +1,4 @@
+MODULE = board
+DIRS = $(RIOTBOARD)/common/nrf52xxxdk


Why is this pointed to the nrf52xxxdk and not nrf52? Is there some refactoring required in those directories?

I was re-using the board.c. Anyhow, fixed.

bergzand · 2020-01-09T15:09:22Z

boards/pinetime/Makefile.include

+TERMFLAGS = term_rtt
+
+# use shared Makefile.include
+include $(RIOTBOARD)/common/nrf52xxxdk/Makefile.include


Same as above, including nrf52xxxdk-files while this is not part of the DK development kits is a bit confusing.

bergzand · 2020-01-09T15:10:22Z

boards/pinetime/doc.txt

+
+"The PINE64 SmartWatch, dubbed "PineTime", is a product of a community effort for an open source smartwatch in collaboration with wearable RTOS and Linux app developers & communities."
+
+See https://wiki.pine64.org/index.php/PineTime#PineTime_DevKit_SWD_Probe for more information.


Why link a specific section of that page here? Isn't it more safe to link to the top of the page (without #PineTime_DevKit_SWD_Probe in the url)

Agreed, that was an accident.

bergzand · 2020-01-09T15:13:16Z

I've got some more code here: https://github.com/kaspar030/pinetime-riot

If I remember correctly that's the branch I based my firmware application on 😆

kaspar030 · 2020-01-09T15:22:06Z

If I remember correctly that's the branch I based my firmware application on laughing

Wow, that's quite advanced! Looking forward to try!

bergzand · 2020-01-09T16:31:35Z

boards/pinetime/include/gpio_params.h

+extern "C" {
+#endif
+
+/* move on, nothing to see here */


Should the backlight GPIO pins also be included here? As an alternative, maybe it is possible to use PWM on them to have more granular brightness control.

Yes, I got all those in my ~~pinetime~~ pinetime_display branch, but not very clean. will update soon!

bergzand · 2020-01-11T21:26:12Z

boards/pinetime/board.c

+    gpio_init(VIBRATOR, GPIO_OUT);
+    gpio_init(LCD_BACKLIGHT_LOW, GPIO_OUT);
+    gpio_init(LCD_BACKLIGHT_MID, GPIO_OUT);
+    gpio_init(LCD_BACKLIGHT_HIGH, GPIO_OUT);


Please be a good neighbor and disable the vibrator, and both LCD_BACKLIGHT_HIGH and LCD_BACKLIGHT_MID with gpio_set(VIBRATOR) (those pins are active low), preferably before initializing them as output if that is possible.

bergzand

2 more comments, feel free to ignore if you think these should wait for a follow up.

boards/pinetime/include/board.h

boards/pinetime/include/periph_conf.h

bergzand · 2020-01-13T10:14:06Z

If I have time this evening I can see if I can also give you a snippet to add config for the SPI flash.

bergzand · 2020-01-13T13:54:42Z

boards/pinetime/Makefile.features

+CPU_MODEL = nrf52832xxaa
+
+# Put defined MCU peripherals here (in alphabetical order)
+#FEATURES_PROVIDED += periph_i2c


This doesn't have to be commented out anymore now that the I2C config is included.

kaspar030 · 2020-01-13T14:29:22Z

addressed all comments

The PR is now able to directly use the ili9341 if the backlight is enabled. E.g., tests/drivers_ili9341 displays something with adding this line: https://gist.github.com/kaspar030/8e252913801e8bb26978093b85dff7f9/raw. I (conditionally) added this to that driver's main.c.
I'd also like to not use the test synchronization for the ili test, if compiling for the pinetime. If used, the pinetime waits on stdio_rtt for 's' indefinitely.

bergzand · 2020-01-13T14:38:11Z

The PR is now able to directly use the ili9341 if the backlight is enabled. E.g., tests/drivers_ili9341 displays something with adding this line: https://gist.github.com/kaspar030/8e252913801e8bb26978093b85dff7f9/raw. I (conditionally) added this to that driver's main.c.
I'd also like to not use the test synchronization for the ili test, if compiling for the pinetime. If used, the pinetime waits on stdio_rtt for 's' indefinitely.

Mind giving #13105 a spin on your PineTime with CFLAGS += -DILI9341_PARAM_INVERTED=1 -DILI9341_PARAM_RGB=1? 😁

kaspar030 · 2020-01-13T14:45:12Z

Mind giving #13105 a spin on your PineTime with CFLAGS += -DILI9341_PARAM_INVERTED=1 -DILI9341_PARAM_RGB=1? grin

will do. the poor thing is going through a compile_and_test run ATM...

kaspar030 · 2020-01-13T16:40:33Z

@bergzand this should be good to go?

aabadie · 2020-01-13T17:44:26Z

boards/pinetime/Makefile.include

@@ -0,0 +1,7 @@
+# for this board, we are using Segger's RTT as default terminal interface
+USEMODULE += stdio_rtt


Please have a look at #12724. That would help make things a little cleaner.

Is this necessary if there's no periph_uart feature, which stdio_uart and ethos depend on?

I think so.

This board is in the same case as the hamilton, so things related to stdio_rtt could be replaced with just:

# use JLink Segger RTT by default RIOT_TERMINAL ?= jlink include $(RIOTMAKE)/tools/serial.inc.mk

And add the following in pinetime/Makefile.dep:

# Use Segger's RTT by default for stdio on this board DEFAULT_MODULE += stdio_rtt

This looks weird: include $(RIOTMAKE)/tools/serial.inc.mk.
The board doesn't have uarts exposed...

Ok, now that I tried I realized that in master, this doesn't work. ;(

boards/common/nrf52xxxdk/board.c

boards/pinetime/include/gpio_params.h

emmanuelsearch · 2020-01-14T14:31:21Z

Tested examples/nimble_gatt and tests/driver_ili9341. Both work like a charm!

kaspar030 · 2020-01-14T14:46:25Z

I've done a full compile_and_test run. Many tests seem to run fine, but stdio_rtt output is pretty broken. Those that fail because of lost output actually succeeded (edit unless blacklisted).

I've managed to get unittests to pass using this patch (adding an early xtimer_init call and increasing stdio_rtt output buffer size).

$ BOARD=pinetime make term
/home/kaspar/src/riot/dist/tools/jlink/jlink.sh term-rtt
### Starting RTT terminal ###
Twisted not available, please install it if you want to use pyterm's JSON capabilities
2020-01-14 15:38:54,102 # Host name for TCP connection is missing, defaulting to "localhost"
2020-01-14 15:38:54,102 # Connect to localhost:19021
Welcome to pyterm!
Type '/exit' to exit.
2020-01-14 15:38:55,108 # SEGGER J-Link V6.56b - Real time terminal output
2020-01-14 15:38:55,108 # SEGGER J-Link EDU V10.1, SN=260107551
2020-01-14 15:38:55,109 # Process: JLinkExe
2020-01-14 15:38:55,109 # main(): This is RIOT! (Version: 2020.01-devel-1724-ge7d34-add_pinetime_support)
2020-01-14 15:38:55,109 # Help: Press s to start test, r to print it is ready
s
2020-01-14 15:38:55,811 # START
2020-01-14 15:39:05,404 # ...............................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................
2020-01-14 15:39:05,404 # OK (959 tests)

Otherwise the tests look fine. The stdio_rtt issues are unrelated to this board.

ping @bergzand, do you see more issues?

@aabadie, regarding the rtt configuration, IMO #12724 is independent of this PR, right?

@fjmolinas please keep an eye on this PR, would be awesome to have the support before fosdem!

bergzand · 2020-01-14T15:40:19Z

I've managed to get unittests to pass using this patch (adding an early xtimer_init call and increasing stdio_rtt output buffer size).

Have you tried compiling stdio_rtt with STDIO_RTT_ENABLE_BLOCKING_STDOUT enabled? Or is that unrelated here?

ping @bergzand, do you see more issues?

I think you've covered everything from my side. Feel free to squash if the others agree.

kaspar030 · 2020-01-14T15:45:02Z

Have you tried compiling stdio_rtt with STDIO_RTT_ENABLE_BLOCKING_STDOUT enabled? Or is that unrelated here?

Yes... It didn't seem to help, there were still some output issues. I think we need to take a close look at stdio_rtt at some point.

aabadie · 2020-01-14T15:49:27Z

If you base your work on #12724 and disable the stdio_rtt module when auto_init is disabled, it will provide a simple workaround to the failing tests.
I managed to do a similar thing in #12304: https://github.com/RIOT-OS/RIOT/pull/12304/files#diff-cc423206e4b071a735d298217a657675R1029

kaspar030 · 2020-01-14T16:11:28Z

If you base your work on #12724 and disable the stdio_rtt module when auto_init is disabled, it will provide a simple workaround to the failing tests.

Talked offline, @aabadie is ok with not applying his suggestion.
IMO, the stdio_rtt issues are unrelated to the pinetime (there are boards in master that suffer the same issues, e.g., the ruuvitag). Also, #12724 can be seen as bugfix to the testing situation, so it is not bound by the feature freeze tomorrow. 😉 So I'll squash this now.

bergzand · 2020-01-14T18:54:10Z

@kaspar030 Murdock has some issues with missing docs for the ILI9341 defines. Feel free to amend directly.

kaspar030 · 2020-01-14T21:18:05Z

@kaspar030 Murdock has some issues with missing docs for the ILI9341 defines. Feel free to amend directly.

Yup, done. Murdock is happy, this needs an ACK!

bergzand

Ack!

bergzand · 2020-01-15T08:51:36Z

Al green, Go!

emmanuelsearch · 2020-01-15T10:04:29Z

Nice!

kaspar030 · 2020-01-15T10:31:53Z

Thanks everyone!

kaspar030 added Type: new feature The issue requests / The PR implemements a new feature for RIOT CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Area: boards Area: Board ports labels Oct 23, 2019

kaspar030 requested review from aabadie and haukepetersen October 23, 2019 11:56

cgundogan reviewed Oct 23, 2019

View reviewed changes

kaspar030 force-pushed the add_pinetime_support branch from db33e00 to e702726 Compare October 23, 2019 12:16

aabadie mentioned this pull request Oct 24, 2019

ili9341: Initial import of ili9341 LCD driver #9948

Merged

bergzand requested changes Jan 9, 2020

View reviewed changes

bergzand reviewed Jan 9, 2020

View reviewed changes

kaspar030 force-pushed the add_pinetime_support branch from 497c267 to 104d0bd Compare January 9, 2020 22:38

bergzand reviewed Jan 11, 2020

View reviewed changes

bergzand reviewed Jan 13, 2020

View reviewed changes

boards/pinetime/include/board.h Show resolved Hide resolved

boards/pinetime/include/periph_conf.h Show resolved Hide resolved

bergzand reviewed Jan 13, 2020

View reviewed changes

aabadie reviewed Jan 13, 2020

View reviewed changes

boards/common/nrf52xxxdk/board.c Outdated Show resolved Hide resolved

aabadie reviewed Jan 13, 2020

View reviewed changes

boards/pinetime/include/gpio_params.h Outdated Show resolved Hide resolved

kaspar030 force-pushed the add_pinetime_support branch from 69f9b63 to e7d34b5 Compare January 13, 2020 21:56

kaspar030 force-pushed the add_pinetime_support branch from e7d34b5 to 2b658c1 Compare January 14, 2020 16:12

kaspar030 added 2 commits January 14, 2020 21:06

boards/pinetime: initial support for the PINE64 PineTime smartwatch

5c21df5

tests/driver_ili9341: enable backlight on PineTime

4c0f4da

kaspar030 force-pushed the add_pinetime_support branch from 2b658c1 to 4c0f4da Compare January 14, 2020 20:06

emmanuelsearch self-requested a review January 15, 2020 08:12

bergzand approved these changes Jan 15, 2020

View reviewed changes

bergzand merged commit fcec592 into RIOT-OS:master Jan 15, 2020

kaspar030 deleted the add_pinetime_support branch January 15, 2020 10:31

fjmolinas added this to the Release 2020.01 milestone Jan 15, 2020

bergzand mentioned this pull request Jan 18, 2020

pinetime: Fix battery ADC line define #13162

Merged

		@@ -0,0 +1,4 @@
		MODULE = board
		DIRS = $(RIOTBOARD)/common/nrf52xxxdk


		"The PINE64 SmartWatch, dubbed "PineTime", is a product of a community effort for an open source smartwatch in collaboration with wearable RTOS and Linux app developers & communities."

		See https://wiki.pine64.org/index.php/PineTime#PineTime_DevKit_SWD_Probe for more information.

		@@ -0,0 +1,7 @@
		# for this board, we are using Segger's RTT as default terminal interface
		USEMODULE += stdio_rtt

Conversation

kaspar030 commented Oct 23, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Contribution description

Testing procedure

Issues/PRs references

Uh oh!

kaspar030 commented Oct 23, 2019

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kaspar030 commented Oct 23, 2019

Uh oh!

bergzand commented Oct 23, 2019

Uh oh!

kaspar030 commented Oct 23, 2019

Uh oh!

bergzand commented Oct 23, 2019

Uh oh!

kaspar030 commented Oct 23, 2019

Uh oh!

patchedsoul commented Dec 27, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kaspar030 commented Jan 9, 2020

Uh oh!

bergzand commented Jan 9, 2020

Uh oh!

kaspar030 commented Jan 9, 2020

Uh oh!

bergzand left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kaspar030 Jan 13, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bergzand commented Jan 9, 2020

Uh oh!

kaspar030 commented Jan 9, 2020

Uh oh!

bergzand Jan 9, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

kaspar030 commented Oct 23, 2019 •

edited

Loading

patchedsoul commented Dec 27, 2019 •

edited

Loading

kaspar030 Jan 13, 2020 •

edited

Loading

bergzand Jan 9, 2020 •

edited

Loading

kaspar030 Jan 9, 2020 •

edited

Loading