Skip to content

Conversation

@hakehuang
Copy link
Contributor

The original samples can not checked by twister only build check. with this PR, the board run testing can be checked by twister, although it can not check the display content, we can check the log and ensure the data flow is running.

Copy link
Contributor

@danieldegrasse danieldegrasse left a comment

Choose a reason for hiding this comment

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

Display changes look ok, @anas could you review the twister addition?

Copy link
Contributor

Choose a reason for hiding this comment

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

We should add some documentation on this new filter feature for twister, it seems like a useful addition

Copy link
Contributor Author

@hakehuang hakehuang Sep 26, 2024

Choose a reason for hiding this comment

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

yes, I need add document. done

@PerMac
Copy link
Member

PerMac commented Sep 26, 2024

from a quick glance it looks like something that could simplify our yamls. It could also help with the issue we discussed today with @nashif during testing meetings, i.e. having dozens of configuration variants for a test/sample and all of them getting separate builds.

@hakehuang I think this PR can (should?) be split or reworked. As I understand, the majority of changes to the display samples has nothing to do with the namespaceing in extra_args. namespacing is only mentioned in docs and used in demo sample. Maybe namespacing could be used also in display sample in this PR? It looks like there are several very similar variants, where the only difference is platform_allow with a single platform and which shield should be used. Would it make sens to combine those into sth like:

sample.subsys.display.lvgl:
  platform_allow:
    - platform_X
    - platform_Y
    - platform_Z
  extra_args:
    platform:platform_X:SHIELD=shield_A
    platform:platform_Y:SHIELD=shield_B
    platform:platform_Z:SHIELD=shield_A

@hakehuang
Copy link
Contributor Author

hakehuang commented Sep 27, 2024

@hakehuang I think this PR can (should?) be split or reworked.

@PerMac I have split this PR in several commits. do you mean to split in to different PR, but the demo sample has a dependency on the runner changes.

sample.subsys.display.lvgl:
  platform_allow:
    - platform_X
    - platform_Y
    - platform_Z
  extra_args:
    platform:platform_X:SHIELD=shield_A
    platform:platform_Y:SHIELD=shield_B
    platform:platform_Z:SHIELD=shield_A

the problem here is the shield and board are not 1to1 mapping, it is one to many. as you can see in display sample, mimxrt595_evk support twi display shield, and above logic can't resolve the conflicts.

in display test, there is a filter called 'zephyr,display', which is obtained by shield board, so platform_allow is not needed, if the shield extra_args is not there.

@danieldegrasse
Copy link
Contributor

danieldegrasse commented Sep 30, 2024

@PerMac I have split this PR in several commits. do you mean to split in to different PR, but the demo sample has a dependency on the runner changes.

I don't want to slow this PR, but I think that the twister feature should probably be in a separate PR. I don't want people who would otherwise review this feature to miss its addition because it is present within a PR targeting display tests

@hakehuang
Copy link
Contributor Author

I don't want to slow this PR, but I think that the twister feature should probably be in a separate PR. I don't want people who would otherwise review this feature to miss its addition because it is present within a PR targeting display tests

I add a PR(#79263) and will update this one once merged

@hakehuang hakehuang marked this pull request as draft October 1, 2024 11:21
@hakehuang hakehuang marked this pull request as ready for review October 16, 2024 04:55
danieldegrasse
danieldegrasse previously approved these changes Oct 16, 2024
add display type for fixtures to avoid conflcts
in NXP platform

1. add required regex for console harness.
2. add message after processing, ensure the flow is OK
3. reduce the test time in TEST mode

Signed-off-by: Hake Huang <[email protected]>
@hakehuang hakehuang force-pushed the nxp_display branch 3 times, most recently from c644de0 to a48dace Compare October 25, 2024 11:50
Copy link
Contributor

Choose a reason for hiding this comment

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

Copyright should be updated- also, won't the display running with this sample on the RT595 use RGB565, not ARGB8888?

1. add lvgl status output after starts
2. add extra_args platfrom filterable
3. add some platfroms shield as extra_args

Signed-off-by: Hake Huang <[email protected]>
@hakehuang hakehuang requested a review from faxe1008 November 20, 2024 03:49
@hakehuang
Copy link
Contributor Author

@kartben @faxe1008 please check thanks.

@nashif nashif merged commit 55e642c into zephyrproject-rtos:main Nov 26, 2024
17 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants