lib: dk_buttons_and_leds: Include nrfx.h to make GPIO defines available #180
Conversation
This patch adds include of nrfx.h to make necessary GPIO defines for available for the DK buttons and LEDs library. Signed-off-by: Jan Tore Guggedal <[email protected]>
|
cc @jhn-nordic @barsok @joakimtoe This is to make the define |
|
@jtguggedal I think it would be better to use macros defined in fixups (i.e. |
| @@ -10,6 +10,7 @@ | |||
| #include <gpio.h> | |||
| #include <misc/util.h> | |||
| #include <logging/sys_log.h> | |||
| #include <nrfx.h> | |||
There was a problem hiding this comment.
CONFIG_GPIO_P0_DEV_NAME is really unavailable? It should be included from fixups brought by soc.h. I think all boards include this file in their board.h file. Maybe this is missing?
There was a problem hiding this comment.
Yes, when compiling for nRF91, CONFIG_GPIO_P0_DEV_NAME is currently not defined. I need to refer to @barsok for an explanation of why this is. I agree that using CONFIG_GPIO_P0_DEV_NAME without the need of any extra header file would be the preferred solution, but as of now I don't think it's possible. CONFIG_GPIO_0_LABEL is defined, but my understanding is that CONFIG_GPIO_P0_DEV_NAME is the preferred way to get the device name.
There was a problem hiding this comment.
it seems that somebody forgot to include a header but in such case it would be better to fix the actual problem. Otherwise people will have to make sure they add this additional include.
There was a problem hiding this comment.
The point was to reduce dts_fixup.h to a minimum. In each compilation autoconfig.h is available (injected in gcc command line). A lot of other includes also include generated_dts_board.h and there are all necessary labels etc. The legacy kconfig names have been moved to nrfx.h. Some discussion may be needed if there is a need to duplicate DTS definitions, ie CONFIG_GPIO_P0_DEV_NAME with CONFIG_GPIO_0_LABEL. If you think so we can discuss what is really needed short-term and long-term
There was a problem hiding this comment.
From what I have seen in zephyr code base then CONFIG_GPIO_P0_DEV_NAME is very often used.
There was a problem hiding this comment.
I don't have strong opinions on this, other than that we need a fix for this as soon as possible so samples using this library can compile.
I'll leave it up to you as the author of the lib to decide which solution to use @jhn-nordic , I will update the PR accordingly.
There was a problem hiding this comment.
@jtguggedal I think you should include nrfx.h and then we can remove it when CONFIG_GPIO_P0_DEV_NAME gets available again.
There was a problem hiding this comment.
Ok, then this PR is merge-ready I think
There was a problem hiding this comment.
@barsok I have created a ticket in Zephyr to improve the way labels are defined. Maybe there will be some further discussion regarding this.
zephyrproject-rtos/zephyr#10938
|
A side comment, has there been any discussion about upstreaming this library? |
It has not been discussed as far as I know. If we are going to use it more extensively there are some things that should be added like support for going to sleep and use interrupt together with the polling that is used now. It also would need to support not only using GPIO_P0 |
|
@carlescufi This one is ready to be merged, and is needed for samples (like nRF Cloud) to compile without errors. The inclusion of nrfx.h introduced in this patch will be removed when there is a way to make these defines available otherwise. |
This commit is automatically created as result of Pull request nrfconnect#180 to nrfconnect/sdk-nrfxlib. Signed-off-by: Nordic Builder <[email protected]>
This commit is automatically created as result of Pull request nrfconnect#180 to nrfconnect/sdk-nrfxlib. Signed-off-by: Nordic Builder <[email protected]>
This commit is automatically created as result of Pull request nrfconnect#180 to nrfconnect/sdk-nrfxlib. Signed-off-by: Nordic Builder <[email protected]>
This commit is automatically created as result of Pull request nrfconnect#180 to nrfconnect/sdk-nrfxlib. Signed-off-by: Nordic Builder <[email protected]>
This commit is automatically created as result of Pull request nrfconnect#180 to nrfconnect/sdk-nrfxlib. Signed-off-by: Nordic Builder <[email protected]>
This commit is automatically created as result of Pull request nrfconnect#180 to nrfconnect/sdk-nrfxlib. Signed-off-by: Nordic Builder <[email protected]>
This commit is automatically created as result of Pull request nrfconnect#180 to nrfconnect/sdk-nrfxlib. Signed-off-by: Nordic Builder <[email protected]>
This commit is automatically created as result of Pull request nrfconnect#180 to nrfconnect/sdk-nrfxlib. Signed-off-by: Nordic Builder <[email protected]>
This commit is automatically created as result of Pull request nrfconnect#180 to nrfconnect/sdk-nrfxlib. Signed-off-by: Nordic Builder <[email protected]>
This patch adds include of nrfx.h to make necessary GPIO defines for
available for the DK buttons and LEDs library.
Signed-off-by: Jan Tore Guggedal [email protected]