Skip to content

Conversation

@danieldegrasse
Copy link
Contributor

Consolidate the testcase.yaml definition for the display sample where possible to use a single testcase for all shields. This won't work for all testcases, but helps avoid the pattern where a new testcase using "platform-allow" is added for every shield/board combination that needs to be run with this sample.

With the merge of #79263, we can now define a sample.display.shield that can capture cases where a board simply needs to build with an attached shield.

This has the advantage of simplifying the display sample testcase definition. However, it reduces coverage for some shields (such as adafruit_2_8_tft_touch_v2) which were previously built on all boards supporting arduino_spi. It also reduces coverage for platforms like the nrf52840dk/nrf52840, that were being built for multiple displays previously.

Not sure if the simplification here is worth those disadvantages, but wanted to put this out to see what others thoughts were.

Consolidate the testcase.yaml definition for the display sample where
possible to use a single testcase for all shields. This won't work for
all testcases, but helps avoid the pattern where a new testcase using
"platform-allow" is added for every shield/board combination that needs
to be run with this sample.

Signed-off-by: Daniel DeGrasse <[email protected]>
- platform:mimxrt1170_evk@B/mimxrt1176/cm7:SHIELD=rk055hdmipi4m
- platform:da1469x_dk_pro:DTC_OVERLAY_FILE=da1469x_dk_pro_mipi_dbi.overlay
- platform:nrf52840dk/nrf52840:SHIELD=max7219_8x8
- platform:stm32h747i_disco/stm32h747xx/m7:SHIELD=st_b_lcd40_dsi1_mb1166
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sample.display.st_b_lcd40_dsi1_mb1166_a09 above can also be removed & added to this list, it's a variant of st_b_lcd40_dsi1_mb1166 with a different panel controller.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think based on my understanding of #79263, we can only map one shield to a given platform. @hakehuang is this correct? That is why some of the shield cases still exist, because in some cases one platform is being built with multiple shields

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@danieldegrasse you mean it's not possible to have a platform in multiple items in the list?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is possible, but it would not make sense as this would add both shields to the same build.

@henrikbrixandersen henrikbrixandersen merged commit e0fe1e4 into zephyrproject-rtos:main Oct 22, 2024
20 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants