Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added dummy display driver #12223

Merged
merged 1 commit into from
Dec 30, 2018

Conversation

vanwinkeljan
Copy link
Member

Added dummy display driver

This driver can be used in CI together with a native posix target

fixes: #12001

Added dummy display driver.

fixes: zephyrproject-rtos#12001

Signed-off-by: Jan Van Winkel <[email protected]>
@codecov-io
Copy link

Codecov Report

Merging #12223 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master   #12223   +/-   ##
=======================================
  Coverage    48.3%    48.3%           
=======================================
  Files         293      293           
  Lines       44136    44136           
  Branches    10591    10591           
=======================================
  Hits        21318    21318           
  Misses      18552    18552           
  Partials     4266     4266

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update bebdf2a...1e88ccd. Read the comment docs.

Copy link
Member

@aescolar aescolar left a comment

Choose a reason for hiding this comment

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

To me, it seems fine, but I'm no display API expert.

@aescolar
Copy link
Member

Sidetrack: It seems a bit inconvenient that, to change the display driver that is initialized one needs to change the application. E.g.:

diff --git a/samples/display/cfb/src/main.c b/samples/display/cfb/src/main.c
index c4e74cbd27..a36b0b0d10 100644
--- a/samples/display/cfb/src/main.c
+++ b/samples/display/cfb/src/main.c
@@ -17,6 +17,10 @@
 #define DISPLAY_DRIVER                         "SSD1306"
 #endif
 
+#if defined (CONFIG_DUMMY_DISPLAY)
+#define DISPLAY_DRIVER                         "DUMMY_DISPLAY"
+#endif
+
 #ifndef DISPLAY_DRIVER
 #define DISPLAY_DRIVER "DISPLAY"
 #endif

@vanwinkeljan
Copy link
Member Author

vanwinkeljan commented Dec 30, 2018

@aescolar Thx for the review!

On the sidetrack:

Sidetrack: It seems a bit inconvenient that, to change the display driver that is initialized one needs to change the application. E.g.:

There is not need to change the code if CONFIG_DUMMY_DISPLAY_DEV_NAME is set to "DISPLAY" instead of the default value ("DUMMY_DISPLAY"). I tried this with the CFB sample + overlay config and it was working fine:

overlay-dummy-display.conf

CONFIG_DUMMY_DISPLAY=y
CONFIG_DUMMY_DISPLAY_DEV_NAME="DISPLAY"
CONFIG_DUMMY_DISPLAY_X_RES=250
CONFIG_DUMMY_DISPLAY_Y_RES=120

CONFIG_SDL_DISPLAY=n

Cmake cmd:

cmake -DOVERLAY_CONFIG="overlay-dummy-display.conf" -DBOARD=native_posix -GNinja ..

@nashif nashif merged commit 848b084 into zephyrproject-rtos:master Dec 30, 2018
@vanwinkeljan vanwinkeljan deleted the dummy_display branch December 30, 2018 21:31
@aescolar
Copy link
Member

aescolar commented Dec 31, 2018

There is not need to change the code

( Yes.. and one could also have set the DISPLAY define [once I had seen in the code that the DISPLAY macro was the problem] from the cmake command line).
But what I was thinking, is that I do not really see the point of these *_DISPLAY_DEV_NAME being anything else than "DISPLAY" by default.
As a side-effect that would avoid needing to do anything more than selecting a driver thru Kconfig

@vanwinkeljan
Copy link
Member Author

But what I was thinking, is that I do not really see the point of these *_DISPLAY_DEV_NAME being anything else than "DISPLAY" by default.

The only reason I can come up with is that if you would have multiple displays you could get a name clash if they all default to "DISPLAY"

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.

Dummy Display driver for native_posix
4 participants