thingy53 rgb_led sample

[tldr]

I'm trying to test the RGB led on the thingy53.

Using the sample:
C:\ncs\v2.6.0\zephyr\samples\basic\rgb_led
which gives errors about the devices error: '__device_dts_ord_DT_N_ALIAS_red_pwm_led_P_pwms_IDX_0_PH_ORD' undeclared
I would have thought those would be defined just as they are on the thingy52.

I found the following which provided some devicetree overlays, which I have tried without success.
https://devzone.nordicsemi.com/f/nordic-q-a/73245/controlling-rgb-led-on-nordic-thingy-52-using-nrf52-sdk

Am I supposed to create an overlay for thingy53 to use the RGB led?  Can you help me get one that works?  I have tried attached.

thingy53_nrf5340_cpuapp.overlay

[Elfving]

Hello David,

So first of all, the rgb_led sample is a sample directly from Zephyr, which means it might not work right out of the box with a Nordic board. Though with something as basic as this it should be an easy fix. 

I found the following which provided some devicetree overlays, which I have tried without success.
https://devzone.nordicsemi.com/f/nordic-q-a/73245/controlling-rgb-led-on-nordic-thingy-52-using-nrf52-sdk

I'm not really seeing the overlay files here, this case seems to be refering to how to do it in the old nRF5SDK, which is not the new Zephyr based nRF Connect SDK.

So if we have a look at the main file of the sample, we see some references to names of nodes and aliases that are not used in the thingys DTS files (red_pwm_led vs red_led_pwm for instance). An overlay file would be used to change the board, and for instance the aliases used so yeah you can make one of those and add this to it:

/ {

    aliases{
		redpwm = &red_led_pwm;
		greenpwm = &green_led_pwm;
		bluepwm = &blue_led_pwm;

    };
};

And you could also change these lines in the main.c file.

static const struct pwm_dt_spec red_pwm_led =
	PWM_DT_SPEC_GET(DT_ALIAS(redpwm));
static const struct pwm_dt_spec green_pwm_led =
	PWM_DT_SPEC_GET(DT_ALIAS(greenpwm));
static const struct pwm_dt_spec blue_pwm_led =
	PWM_DT_SPEC_GET(DT_ALIAS(bluepwm));

For more info how to work with overlay files, dts etc.I would recommend having a look in our DevAcademy.

Regards,

Elfving

[tldr]

I just changed the overlay to be just what you supplied and I also changed thee names in DT_ALIAS().

C:/ncs/v2.6.0/zephyr/include/zephyr/device.h:89:41: error: '__device_dts_ord_DT_N_ALIAS_redpwm_P_pwms_IDX_0_PH_ORD' undeclared here (not in a function)
89 | #define DEVICE_NAME_GET(dev_id) _CONCAT(__device_, dev_id)
| ^~~~~~~~~
C:/ncs/v2.6.0/zephyr/include/zephyr/toolchain/common.h:137:26: note: in definition of macro '_DO_CONCAT'
137 | #define _DO_CONCAT(x, y) x ## y

Still having trouble with the sample.  thanks for your quick response.

[tldr]

Using the same board overlay and code I tried to use the thingy91.
west build -b thingy91x_nrf5340_cpuapp . --build-dir thingy91build

C:/ncs/v2.6.1/zephyr/include/zephyr/device.h:89:41: error: '__device_dts_ord_DT_N_ALIAS_bluepwm_P_pwms_IDX_0_PH_ORD' undeclared here (not in a function)
89 | #define DEVICE_NAME_GET(dev_id) _CONCAT(__device_, dev_id)

the other i2c rtc device is also showing similar. 

It looks like they are defined as led0-2; I set those alias.

aliases {

        redpwm = &led0;

        greenpwm = &led1;

        bluepwm = &led2;

}

Doing that clears more of the errors. But still leads to 

C:/ncs/v2.6.1/zephyr/include/zephyr/device.h:89:41: error: '__device_dts_ord_44' undeclared here (not in a function); did you mean '__device_dts_ord_46'?
   89 | #define DEVICE_NAME_GET(dev_id) _CONCAT(__device_, dev_id)
      |                                         ^~~~~~~~~
C:/ncs/v2.6.1/zephyr/include/zephyr/toolchain/common.h:137:26: note: in definition of macro '_DO_CONCAT'
  137 | #define _DO_CONCAT(x, y) x ## y
      |                          ^
C:/ncs/v2.6.1/zephyr/include/zephyr/device.h:89:33: note: in expansion of macro '_CONCAT'
   89 | #define DEVICE_NAME_GET(dev_id) _CONCAT(__device_, dev_id)
      |                                 ^~~~~~~
C:/ncs/v2.6.1/zephyr/include/zephyr/device.h:216:37: note: in expansion of macro 'DEVICE_NAME_GET'
  216 | #define DEVICE_DT_NAME_GET(node_id) DEVICE_NAME_GET(Z_DEVICE_DT_DEV_ID(node_id))

I need to spend more time with the NCS docs you pointed at; I did define an overlay the same as the 53 and attempt it from within vscode as well.

Should the thingy91 require a different overlay to support the rgb leds?  I'm surprised the thingy53 required the custom device overlay to get the rgb sample working.  Looks like I will have to maintain separate overlays for the 53 and 91?

[Elfving]

Ah yes the different boards are made a bit differently, which means that you'll unfortunately have to work a bit on these overlay files to get things working. 

tldr said:

I'm surprised the thingy53 required the custom device overlay to get the rgb sample working.

Like I said, this being something you have to do is because it comes directly from Zephyr. The samples made my Nordic should work by default on all of our boards (.. or atleast the ones we list as supported by a certain sample, though it should work on more than what is listed here but we only list the ones we've tested). 

The Thingy91 really stands out, as the nRF9160 can't do any BLE on its own, which is why we give the board an nRF5340 as well. And now you have two nRFs on one PCB, with some LEDs connected to the one, and some to the other. The dts file is bound to be a bit different.

As a starting point in order to get familiar with NCS it might be easier to work with samples from us, but also DKs instead of Thingys.

I can look further into this if you want, but that is the cause of the difference between the Thingy53 and 91.

Regards and have a good week-end!

Elfving

[tldr]

Its definitely more complicated than just doing aliases.  I can focus on the 53; but I'm interested in the thingys and not the DK boards at this time.

The first error is cryptic and I'm not sure 
C:/ncs/v2.6.1/zephyr/include/zephyr/device.h:89:41: error: '__device_dts_ord_44' undeclared here (not in a function); did you mean '__device_dts_ord_46'?
89 | #define DEVICE_NAME_GET(dev_id) _CONCAT(__device_, dev_id)
| ^~~~~~~~~
C:/ncs/v2.6.1/zephyr/include/zephyr/toolchain/common.h:137:26: note: in definition of macro '_DO_CONCAT'
137 | #define _DO_CONCAT(x, y) x ## y

looking at the dts for the 91 I don't see any i2c defined either.  Based on your details about the two chips and the lack of BLE on one vs the other; sounds like the thingy91 is quite a bit of work to get the same code working on both.  Again will need to define the buzzer that does not get defined either.

I understand it is not a sample from nordic; would be nice to use names that match upstream so that simple examples just work (at least for the rgb; its like hello world).

[Elfving]

I guess you'll have to start off finding the leds you want to use in the Thingys on the DK, and find what SoC you want to program for controlling it. By flashing thingy91x_nrf5340_cpuapp you are targeting the nRF5340 SoC on the board, but you could also program the nRF9160 if you wanted to. 

tldr said:

looking at the dts for the 91 I don't see any i2c defined either.  Based on your details about the two chips and the lack of BLE on one vs the other; sounds like the thingy91 is quite a bit of work to get the same code working on both. 

I might be misunderstanding you here. Though you can have a look at thingy91_nrf9160_common.dts for the i2c. For how to get it working on the Thing91, I see there's been a previous case about this here that you can have a look at.

tldr said:

I understand it is not a sample from nordic; would be nice to use names that match upstream so that simple examples just work (at least for the rgb; its like hello world).

With Blinky I agree, that should be easy to get supported on all boards right out of the box. But not all boards have rgb leds for instance

Have a good week-end! 

Regards,

Elfving

[tldr]

I still haven't spent the time to figure out how to make the needed changes to make this work (thingy91 is low on my priority list and this is basic sample);

I think the alias to make this work should be added to the default overlay for the thingy devices; both devices have a buzzer and an alias for that would be nice too.  the buzzer less so important; but if we're fixing overlay files... 

the upstream "hello world" of rgb leds should work for beginners out of the box;  if I go to the effort of fixing this for the 91, I'd like to submit a PR.  What repo would that be?

If you have the fix, then someone looking for the rgb_led-sample forum post and find this.

[Elfving]

tldr said:

the upstream "hello world" of rgb leds should work for beginners out of the box;  if I go to the effort of fixing this for the 91, I'd like to submit a PR.  What repo would that be?

The Thingy53 is part of the Upstream Zephyr, so this the PR for this, would go to the same repo, given that the RGB sample is in Zephyr.
However, the Thingy91 is part of the Nordic repo, so it would not be clean to make a board overlay in Zephyr for a board that is not available in zephyr by default. We got our own fork of zephyr over at sdk-zephyr, but we try to keep this as similar to upstream zephyr as possible.

Because of this, I would not suggest adding Thingy91 support to any Zephyr samples, but rather suggest that Thingy91 users get started with samples from sdk-nrf instad.

Regards,

Elfving

[tldr]

I still dont have a thingy91 solution available for this sample. I will comment here if I attempt it again.