Mir

Merge lp:~vanvugt/mir/output-model into lp:mir

  1. output-model
  2. Merge into development-branch
Proposed by Daniel van Vugt
Status: Merged
Approved by: Daniel van Vugt
Approved revision: no longer in the source branch.
Merged at revision: 3882
Proposed branch: lp:~vanvugt/mir/output-model
Merge into: lp:mir
Diff against target: 538 lines (+404/-0)
13 files modified
include/client/mir_toolkit/mir_display_configuration.h (+12/-0)
src/client/display_configuration_api.cpp (+25/-0)
src/client/symbols.map (+1/-0)
src/common/CMakeLists.txt (+1/-0)
src/common/edid.cpp (+67/-0)
src/common/symbols.map (+7/-0)
src/include/common/mir/graphics/edid.h (+111/-0)
src/protobuf/mir_protobuf.proto (+1/-0)
src/server/report/logging/display_configuration_report.cpp (+17/-0)
src/utils/out.c (+4/-0)
tests/acceptance-tests/test_new_display_configuration.cpp (+84/-0)
tests/unit-tests/CMakeLists.txt (+1/-0)
tests/unit-tests/test_edid.cpp (+73/-0)
To merge this branch: bzr merge lp:~vanvugt/mir/output-model
Reviewer Review Type Date Requested Status
Mir CI Bot continuous-integration Approve
Alan Griffiths Needs Fixing
Cemil Azizoglu (community) Approve
Mir development team Pending
Review via email: mp+312880@code.launchpad.net

This proposal supersedes a proposal from 2016-12-01.

Commit message

Add descriptive strings for the connected monitor, if available.

To post a comment you must log in.
Revision history for this message
Mir CI Bot (mir-ci-bot) wrote : Posted in a previous version of this proposal

PASSED: Continuous integration, rev:3864
https://mir-jenkins.ubuntu.com/job/mir-ci/2299/
Executed test runs:
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-mir/2992
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-0-fetch/3057
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=vivid+overlay/3049
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=xenial+overlay/3049
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=yakkety/3049
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=yakkety/3021
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=yakkety/3021/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=xenial+overlay/3021
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=xenial+overlay/3021/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=yakkety/3021
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=yakkety/3021/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=android,release=vivid+overlay/3021
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=android,release=vivid+overlay/3021/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=android,release=vivid+overlay/3021
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=android,release=vivid+overlay/3021/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=mesa,release=xenial+overlay/3021
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=mesa,release=xenial+overlay/3021/artifact/output/*zip*/output.zip

Click here to trigger a rebuild:
https://mir-jenkins.ubuntu.com/job/mir-ci/2299/rebuild

review: Approve (continuous-integration)
Revision history for this message
Mir CI Bot (mir-ci-bot) wrote : Posted in a previous version of this proposal

PASSED: Continuous integration, rev:3865
https://mir-jenkins.ubuntu.com/job/mir-ci/2300/
Executed test runs:
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-mir/2993
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-0-fetch/3058
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=vivid+overlay/3050
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=xenial+overlay/3050
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=yakkety/3050
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=yakkety/3022
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=yakkety/3022/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=xenial+overlay/3022
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=xenial+overlay/3022/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=yakkety/3022
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=yakkety/3022/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=android,release=vivid+overlay/3022
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=android,release=vivid+overlay/3022/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=android,release=vivid+overlay/3022
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=android,release=vivid+overlay/3022/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=mesa,release=xenial+overlay/3022
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=mesa,release=xenial+overlay/3022/artifact/output/*zip*/output.zip

Click here to trigger a rebuild:
https://mir-jenkins.ubuntu.com/job/mir-ci/2300/rebuild

review: Approve (continuous-integration)
Revision history for this message
Daniel van Vugt (vanvugt) wrote : Posted in a previous version of this proposal

Hmm, I might have gone overboard with the fallback logic. That's needed apparently on laptops and tablets which don't provide a nice "monitor name". There's a risk though that the fallback vendor and product codes are just confusing to people. Although they're useful to me as someone who likes to modify/fix laptop screens, that's not a user-friendly activity.

Revision history for this message
Cemil Azizoglu (cemil-azizoglu) wrote : Posted in a previous version of this proposal

Looks good other than the typo ('nul' instead of 'null')

+ * \returns A nul-terminated string or NULL if none available. This string

review: Approve
Revision history for this message
Daniel van Vugt (vanvugt) wrote : Posted in a previous version of this proposal

That's not a typo. "nul" refers to character '\0' to distinguish it from "NULL" which is a pointer.

  https://en.wikipedia.org/wiki/Null_character

Although Google suggests the industry has changed in recent years and often likes to spell it as "null" these days. I don't mind too much either way but "nul" with a single L is historically the more accurate term when referring to character zero.

Revision history for this message
Alan Griffiths (alan-griffiths) wrote : Posted in a previous version of this proposal

Mild "needs fixing" for the magic numbers (in particular there's a relationship between 13 and 14 that ought to be explicit).

review: Needs Fixing
Revision history for this message
Cemil Azizoglu (cemil-azizoglu) wrote : Posted in a previous version of this proposal

> That's not a typo. "nul" refers to character '\0' to distinguish it from
> "NULL" which is a pointer.
>
> https://en.wikipedia.org/wiki/Null_character
>
> Although Google suggests the industry has changed in recent years and often
> likes to spell it as "null" these days. I don't mind too much either way but
> "nul" with a single L is historically the more accurate term when referring to
> character zero.

Didn't know that. Thanks.

Revision history for this message
Mir CI Bot (mir-ci-bot) wrote : Posted in a previous version of this proposal

FAILED: Continuous integration, rev:3868
https://mir-jenkins.ubuntu.com/job/mir-ci/2341/
Executed test runs:
    FAILURE: https://mir-jenkins.ubuntu.com/job/build-mir/3055/console
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-0-fetch/3121
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=vivid+overlay/3113
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=xenial+overlay/3113
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=yakkety/3113
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=yakkety/3084
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=yakkety/3084/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=xenial+overlay/3084
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=xenial+overlay/3084/artifact/output/*zip*/output.zip
    FAILURE: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=yakkety/3084/console
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=android,release=vivid+overlay/3084
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=android,release=vivid+overlay/3084/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=android,release=vivid+overlay/3084
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=android,release=vivid+overlay/3084/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=mesa,release=xenial+overlay/3084
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=mesa,release=xenial+overlay/3084/artifact/output/*zip*/output.zip

Click here to trigger a rebuild:
https://mir-jenkins.ubuntu.com/job/mir-ci/2341/rebuild

review: Needs Fixing (continuous-integration)
Revision history for this message
Daniel van Vugt (vanvugt) wrote : Posted in a previous version of this proposal

^^^
Bug 1616312

Revision history for this message
Mir CI Bot (mir-ci-bot) wrote : Posted in a previous version of this proposal

PASSED: Continuous integration, rev:3869
https://mir-jenkins.ubuntu.com/job/mir-ci/2345/
Executed test runs:
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-mir/3059
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-0-fetch/3125
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=vivid+overlay/3117
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=xenial+overlay/3117
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=yakkety/3117
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=yakkety/3088
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=yakkety/3088/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=xenial+overlay/3088
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=xenial+overlay/3088/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=yakkety/3088
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=yakkety/3088/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=android,release=vivid+overlay/3088
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=android,release=vivid+overlay/3088/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=android,release=vivid+overlay/3088
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=android,release=vivid+overlay/3088/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=mesa,release=xenial+overlay/3088
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=mesa,release=xenial+overlay/3088/artifact/output/*zip*/output.zip

Click here to trigger a rebuild:
https://mir-jenkins.ubuntu.com/job/mir-ci/2345/rebuild

review: Approve (continuous-integration)
Revision history for this message
Alan Griffiths (alan-griffiths) wrote : Posted in a previous version of this proposal

54 and 4 are still "magic"

review: Abstain
Revision history for this message
Mir CI Bot (mir-ci-bot) wrote :

FAILED: Continuous integration, rev:3886
https://mir-jenkins.ubuntu.com/job/mir-ci/2354/
Executed test runs:
    FAILURE: https://mir-jenkins.ubuntu.com/job/build-mir/3069/console
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-0-fetch/3135
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=vivid+overlay/3127
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=xenial+overlay/3127
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=yakkety/3127
    FAILURE: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=yakkety/3098/console
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=xenial+overlay/3098
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=xenial+overlay/3098/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=yakkety/3098
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=yakkety/3098/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=android,release=vivid+overlay/3098
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=android,release=vivid+overlay/3098/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=android,release=vivid+overlay/3098
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=android,release=vivid+overlay/3098/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=mesa,release=xenial+overlay/3098
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=mesa,release=xenial+overlay/3098/artifact/output/*zip*/output.zip

Click here to trigger a rebuild:
https://mir-jenkins.ubuntu.com/job/mir-ci/2354/rebuild

review: Needs Fixing (continuous-integration)
Revision history for this message
Mir CI Bot (mir-ci-bot) wrote :

PASSED: Continuous integration, rev:3887
https://mir-jenkins.ubuntu.com/job/mir-ci/2356/
Executed test runs:
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-mir/3071
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-0-fetch/3137
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=vivid+overlay/3129
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=xenial+overlay/3129
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=yakkety/3129
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=yakkety/3100
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=yakkety/3100/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=xenial+overlay/3100
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=xenial+overlay/3100/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=yakkety/3100
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=yakkety/3100/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=android,release=vivid+overlay/3100
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=android,release=vivid+overlay/3100/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=android,release=vivid+overlay/3100
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=android,release=vivid+overlay/3100/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=mesa,release=xenial+overlay/3100
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=mesa,release=xenial+overlay/3100/artifact/output/*zip*/output.zip

Click here to trigger a rebuild:
https://mir-jenkins.ubuntu.com/job/mir-ci/2356/rebuild

review: Approve (continuous-integration)
Revision history for this message
Cemil Azizoglu (cemil-azizoglu) wrote :

Lots of fields

review: Approve
Revision history for this message
Alan Griffiths (alan-griffiths) wrote :

+struct EDID

1. I don't see any advantage to having the implementation inline. Can we put it in a .cpp file?

2. Having an UPPERCASE name is potentially confusing - we use that for MACROS.

review: Needs Fixing
Revision history for this message
Mir CI Bot (mir-ci-bot) wrote :

PASSED: Continuous integration, rev:3889
https://mir-jenkins.ubuntu.com/job/mir-ci/2368/
Executed test runs:
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-mir/3085
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-0-fetch/3152
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=vivid+overlay/3144
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=xenial+overlay/3144
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=yakkety/3144
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=yakkety/3114
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=yakkety/3114/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=xenial+overlay/3114
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=xenial+overlay/3114/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=yakkety/3114
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=yakkety/3114/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=android,release=vivid+overlay/3114
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=android,release=vivid+overlay/3114/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=android,release=vivid+overlay/3114
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=android,release=vivid+overlay/3114/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=mesa,release=xenial+overlay/3114
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=mesa,release=xenial+overlay/3114/artifact/output/*zip*/output.zip

Click here to trigger a rebuild:
https://mir-jenkins.ubuntu.com/job/mir-ci/2368/rebuild

review: Approve (continuous-integration)
Revision history for this message
Mir CI Bot (mir-ci-bot) wrote :

PASSED: Continuous integration, rev:3890
https://mir-jenkins.ubuntu.com/job/mir-ci/2369/
Executed test runs:
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-mir/3087
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-0-fetch/3154
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=vivid+overlay/3146
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=xenial+overlay/3146
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=yakkety/3146
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=yakkety/3116
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=yakkety/3116/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=xenial+overlay/3116
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=xenial+overlay/3116/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=yakkety/3116
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=yakkety/3116/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=android,release=vivid+overlay/3116
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=android,release=vivid+overlay/3116/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=android,release=vivid+overlay/3116
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=android,release=vivid+overlay/3116/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=mesa,release=xenial+overlay/3116
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=mesa,release=xenial+overlay/3116/artifact/output/*zip*/output.zip

Click here to trigger a rebuild:
https://mir-jenkins.ubuntu.com/job/mir-ci/2369/rebuild

review: Approve (continuous-integration)
Revision history for this message
Alan Griffiths (alan-griffiths) wrote :

*Needs Discussion*

Should Edid belong in namespace mir::graphics in preference to namespace mir?

Revision history for this message
Daniel van Vugt (vanvugt) wrote :

I think the graphics namespace is a bit pointless (or at least needs a better name). The whole project is about graphics so saying something is in the "graphics" namespace does not really contribute to an understanding of the code, or to namespace safety.

Generally I think mir::graphics:: should be abolished unless someone can think of a better name that justifies its existence.

Revision history for this message
Alan Griffiths (alan-griffiths) wrote :

> I think the graphics namespace is a bit pointless (or at least needs a better
> name). The whole project is about graphics so saying something is in the
> "graphics" namespace does not really contribute to an understanding of the
> code, or to namespace safety.
>
> Generally I think mir::graphics:: should be abolished unless someone can think
> of a better name that justifies its existence.

Changing our long standing namespaces isn't part of this MP.

review: Needs Fixing
Revision history for this message
Kevin DuBois (kdub) wrote :

I suppose that edid fits in the graphics namespace of mir better than any other namespace. mir:: is more for common stuff, and it doesn't fit in input, frontend, shell, compositor, etc

Revision history for this message
Chris Halse Rogers (raof) wrote :

+1 on mir::graphics being the sensible spot for it.

213 + enum { minimum_size = 128 };
This would be more idiomatic as static constexpr, but that's non-blocking.

Either here, or in future work, it would make sense to validate the checksum on the EDID. This could be done in a static “constructor” in Edid::. Which would also be nice, rather than have the expectation that you reinterpret_cast<> a bunch of bytes into a mg::Edid.

Revision history for this message
Daniel van Vugt (vanvugt) wrote :

I considered header and checksum validation and decided intentionally that would be a bad idea. Buggy hardware exists, and we should support it as well as we can, even when they fail to checksum their EDID correctly.

This is based on seeing many logs over the years in which users' displays provided invalid EDIDs. We can't fix all hardware, so we should go best effort instead of assuming that no EDID is better than a badly formed EDID.

Revision history for this message
Daniel van Vugt (vanvugt) wrote :

Also I intentionally wrote this code to deal with any form or badly formed EDID already.

Revision history for this message
Mir CI Bot (mir-ci-bot) wrote :

PASSED: Continuous integration, rev:3892
https://mir-jenkins.ubuntu.com/job/mir-ci/2390/
Executed test runs:
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-mir/3116
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-0-fetch/3183
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=vivid+overlay/3175
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=xenial+overlay/3175
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=yakkety/3175
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=yakkety/3145
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=yakkety/3145/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=xenial+overlay/3145
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=xenial+overlay/3145/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=yakkety/3145
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=yakkety/3145/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=android,release=vivid+overlay/3145
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=android,release=vivid+overlay/3145/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=android,release=vivid+overlay/3145
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=android,release=vivid+overlay/3145/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=mesa,release=xenial+overlay/3145
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=mesa,release=xenial+overlay/3145/artifact/output/*zip*/output.zip

Click here to trigger a rebuild:
https://mir-jenkins.ubuntu.com/job/mir-ci/2390/rebuild

review: Approve (continuous-integration)
Revision history for this message
Daniel van Vugt (vanvugt) wrote :

Alright... two weeks later. I've now resolved all the stylistic complaints raised. That's enough. Trying to land it now.

Revision history for this message
Chris Halse Rogers (raof) wrote :

I don't mean fail to parse the EDID on checksum failures, but to expose that the EDID is broken. For some things (particularly modes) a broken EDID *is* worse than no EDID.

Sometimes the checksum fails because the manufacturer failed to compute the checksum correctly. More often it's broken because the EDID, or the DDC pins, or something is actually broken and should be trusted less.

Hm. While I think of it, checking that all the strings we return are printable ascii is probably a sensible thing to do, particularly if we're going to accept known-bad EDIDs

Revision history for this message
Mir CI Bot (mir-ci-bot) :
review: Approve (continuous-integration)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'include/client/mir_toolkit/mir_display_configuration.h'
2--- include/client/mir_toolkit/mir_display_configuration.h 2016-12-08 04:08:06 +0000
3+++ include/client/mir_toolkit/mir_display_configuration.h 2016-12-14 04:10:39 +0000
4@@ -373,6 +373,18 @@
5 void mir_output_disable(MirOutput* output);
6
7 /**
8+ * Get a descriptive manufacturer/model string for the connected display.
9+ * The format of this string is arbitrary and driver-specific but should be
10+ * human-readable and helpful for someone to identify which physical display
11+ * this is. Note this function is not called get_name because that would imply
12+ * the returned value is different for each output, whereas it may not be.
13+ *
14+ * \returns A nul-terminated string or NULL if none available. This string
15+ * remains valid for the lifetime of the MirOutput object.
16+ */
17+char const* mir_output_get_model(MirOutput const* output);
18+
19+/**
20 * Get the physical width of the connected display, in millimetres.
21 *
22 * A best-effort report of the physical width of the display connected to this
23
24=== modified file 'src/client/display_configuration_api.cpp'
25--- src/client/display_configuration_api.cpp 2016-12-08 04:08:06 +0000
26+++ src/client/display_configuration_api.cpp 2016-12-14 04:10:39 +0000
27@@ -21,6 +21,7 @@
28 #include "mir/output_type_names.h"
29 #include "display_configuration.h"
30 #include "mir/uncaught.h"
31+#include "mir/graphics/edid.h"
32
33 namespace mcl = mir::client;
34 namespace mp = mir::protobuf;
35@@ -86,6 +87,30 @@
36 output->set_used(0);
37 }
38
39+char const* mir_output_get_model(MirOutput const* output)
40+{
41+ // In future this might be provided by the server itself...
42+ if (output->has_model())
43+ return output->model().c_str();
44+
45+ // But if not we use the same member for caching our EDID probe...
46+ using mir::graphics::Edid;
47+ if (mir_output_get_edid_size(output) >= Edid::minimum_size)
48+ {
49+ auto edid = reinterpret_cast<Edid const*>(mir_output_get_edid(output));
50+ Edid::MonitorName name;
51+ if (!edid->get_monitor_name(name))
52+ {
53+ auto len = edid->get_manufacturer(name);
54+ snprintf(name+len, sizeof(name)-len, " %hu", edid->product_code());
55+ }
56+ const_cast<MirOutput*>(output)->set_model(name);
57+ return output->model().c_str();
58+ }
59+
60+ return nullptr;
61+}
62+
63 int mir_display_config_get_max_simultaneous_outputs(MirDisplayConfig const* config)
64 {
65 return config->display_card(0).max_simultaneous_outputs();
66
67=== modified file 'src/client/symbols.map'
68--- src/client/symbols.map 2016-12-13 02:50:57 +0000
69+++ src/client/symbols.map 2016-12-14 04:10:39 +0000
70@@ -502,4 +502,5 @@
71 mir_touchpad_configuration_set_middle_mouse_button_emulation;
72 mir_touchpad_configuration_set_scroll_modes;
73 mir_touchpad_configuration_set_tap_to_click;
74+ mir_output_get_model;
75 } MIR_CLIENT_0.25;
76
77=== modified file 'src/common/CMakeLists.txt'
78--- src/common/CMakeLists.txt 2016-11-15 23:48:01 +0000
79+++ src/common/CMakeLists.txt 2016-12-14 04:10:39 +0000
80@@ -22,6 +22,7 @@
81 ${CMAKE_CURRENT_SOURCE_DIR}/libname.cpp ${PROJECT_SOURCE_DIR}/include/common/mir/libname.h
82 ${PROJECT_SOURCE_DIR}/include/common/mir/posix_rw_mutex.h
83 posix_rw_mutex.cpp
84+ edid.cpp
85 )
86
87 set(PREFIX "${CMAKE_INSTALL_PREFIX}")
88
89=== added file 'src/common/edid.cpp'
90--- src/common/edid.cpp 1970-01-01 00:00:00 +0000
91+++ src/common/edid.cpp 2016-12-14 04:10:39 +0000
92@@ -0,0 +1,67 @@
93+/*
94+ * Copyright © 2016 Canonical Ltd.
95+ *
96+ * This program is free software: you can redistribute it and/or modify it
97+ * under the terms of the GNU Lesser General Public License version 3,
98+ * as published by the Free Software Foundation.
99+ *
100+ * This program is distributed in the hope that it will be useful,
101+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
102+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
103+ * GNU Lesser General Public License for more details.
104+ *
105+ * You should have received a copy of the GNU Lesser General Public License
106+ * along with this program. If not, see <http://www.gnu.org/licenses/>.
107+ *
108+ * Authored by: Daniel van Vugt <daniel.van.vugt@canonical.com>
109+ */
110+
111+#include "mir/graphics/edid.h"
112+#include <endian.h>
113+#include <cstring>
114+
115+using mir::graphics::Edid;
116+
117+size_t Edid::get_monitor_name(MonitorName str) const
118+{
119+ size_t len = get_string(string_monitor_name, str);
120+ if (char* pad = strchr(str, '\n'))
121+ {
122+ *pad = '\0';
123+ len = pad - str;
124+ }
125+ return len;
126+}
127+
128+size_t Edid::get_manufacturer(Manufacturer str) const
129+{
130+ // Confusingly this field is more like big endian. Others are little.
131+ auto man = static_cast<uint16_t>(manufacturer[0]) << 8 | manufacturer[1];
132+ str[0] = ((man >> 10) & 31) + 'A' - 1;
133+ str[1] = ((man >> 5) & 31) + 'A' - 1;
134+ str[2] = (man & 31) + 'A' - 1;
135+ str[3] = '\0';
136+ return 3;
137+}
138+
139+uint16_t Edid::product_code() const
140+{
141+ return le16toh(product_code_le);
142+}
143+
144+size_t Edid::get_string(StringDescriptorType type, char str[14]) const
145+{
146+ size_t len = 0;
147+ for (int d = 0; d < 4; ++d)
148+ {
149+ auto& desc = descriptor[d];
150+ if (!desc.other.zero0 && desc.other.type == type)
151+ {
152+ len = sizeof desc.other.text;
153+ memcpy(str, desc.other.text, len);
154+ break;
155+ }
156+ }
157+ str[len] = '\0';
158+ return len;
159+}
160
161=== modified file 'src/common/symbols.map'
162--- src/common/symbols.map 2016-11-15 23:48:01 +0000
163+++ src/common/symbols.map 2016-12-14 04:10:39 +0000
164@@ -403,3 +403,10 @@
165 mir::PosixRWMutex::unlock_shared*;
166 };
167 } MIR_COMMON_0.25;
168+
169+MIR_COMMON_0.26 {
170+ global:
171+ extern "C++" {
172+ mir::graphics::Edid::*;
173+ };
174+} MIR_COMMON_0.25;
175
176=== added directory 'src/include/common/mir/graphics'
177=== added file 'src/include/common/mir/graphics/edid.h'
178--- src/include/common/mir/graphics/edid.h 1970-01-01 00:00:00 +0000
179+++ src/include/common/mir/graphics/edid.h 2016-12-14 04:10:39 +0000
180@@ -0,0 +1,111 @@
181+/*
182+ * Copyright © 2016 Canonical Ltd.
183+ *
184+ * This program is free software: you can redistribute it and/or modify it
185+ * under the terms of the GNU Lesser General Public License version 3,
186+ * as published by the Free Software Foundation.
187+ *
188+ * This program is distributed in the hope that it will be useful,
189+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
190+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
191+ * GNU Lesser General Public License for more details.
192+ *
193+ * You should have received a copy of the GNU Lesser General Public License
194+ * along with this program. If not, see <http://www.gnu.org/licenses/>.
195+ *
196+ * Authored by: Daniel van Vugt <daniel.van.vugt@canonical.com>
197+ */
198+
199+#ifndef MIR_GRAPHICS_EDID_H_
200+#define MIR_GRAPHICS_EDID_H_
201+
202+#include <cstdint>
203+#include <cstddef>
204+
205+namespace mir { namespace graphics {
206+
207+struct Edid
208+{
209+ Edid() = delete;
210+ Edid(Edid const&) = delete;
211+ Edid(Edid const&&) = delete;
212+
213+ enum { minimum_size = 128 };
214+ typedef char MonitorName[14]; // up to 13 characters
215+ typedef char Manufacturer[4]; // always 3 characters
216+
217+ size_t get_monitor_name(MonitorName str) const;
218+ size_t get_manufacturer(Manufacturer str) const;
219+ uint16_t product_code() const;
220+
221+private:
222+ /* Pretty much every field in an EDID requires some kind of conversion
223+ and reinterpretation. So keep those details private... */
224+
225+ enum StringDescriptorType
226+ {
227+ string_monitor_serial_number = 0xff,
228+ string_unspecified_text = 0xfe,
229+ string_monitor_name = 0xfc,
230+ };
231+
232+ size_t get_string(StringDescriptorType type, char str[14]) const;
233+
234+ union Descriptor
235+ {
236+ struct
237+ {
238+ uint16_t pixel_clock_le;
239+ uint8_t todo[16];
240+ } detailed_timing;
241+ struct
242+ {
243+ uint16_t zero0;
244+ uint8_t zero2;
245+ uint8_t type;
246+ uint8_t zero4;
247+ uint8_t text[13];
248+ } other;
249+ };
250+
251+#ifdef __clang__
252+#pragma clang diagnostic push
253+#pragma clang diagnostic ignored "-Wunused-private-field"
254+#endif
255+ /* 0x00 */ uint8_t header[8];
256+ /* 0x08 */ uint8_t manufacturer[2];
257+ /* 0x0a */ uint16_t product_code_le;
258+ /* 0x0c */ uint32_t serial_number_le;
259+ /* 0x10 */ uint8_t week_of_manufacture;
260+ /* 0x11 */ uint8_t year_of_manufacture;
261+ /* 0x12 */ uint8_t edid_version;
262+ /* 0x13 */ uint8_t edid_revision;
263+ /* 0x14 */ uint8_t input_bitmap;
264+ /* 0x15 */ uint8_t max_horz_cm;
265+ /* 0x16 */ uint8_t max_vert_cm;
266+ /* 0x17 */ uint8_t gamma;
267+ /* 0x18 */ uint8_t features_bitmap;
268+ /* 0x19 */ uint8_t red_green_bits_1to0;
269+ /* 0x1a */ uint8_t blue_white_bits_1to0;
270+ /* 0x1b */ uint8_t red_x_bits_9to2;
271+ /* 0x1c */ uint8_t red_y_bits_9to2;
272+ /* 0x1d */ uint8_t green_x_bits_9to2;
273+ /* 0x1e */ uint8_t green_y_bits_9to2;
274+ /* 0x1f */ uint8_t blue_x_bits_9to2;
275+ /* 0x20 */ uint8_t blue_y_bits_9to2;
276+ /* 0x21 */ uint8_t white_x_bits_9to2;
277+ /* 0x22 */ uint8_t white_y_bits_9to2;
278+ /* 0x23 */ uint8_t established_timings[2];
279+ /* 0x25 */ uint8_t reserved_timings;
280+ /* 0x26 */ uint8_t standard_timings[2][8];
281+ /* 0x36 */ Descriptor descriptor[4];
282+ /* 0x7e */ uint8_t num_extensions; /* each is another 128-byte block */
283+ /* 0x7f */ uint8_t checksum;
284+#ifdef __clang__
285+#pragma clang diagnostic pop
286+#endif
287+};
288+
289+} } // namespace mir::graphics
290+
291+#endif // MIR_GRAPHICS_EDID_H_
292
293=== modified file 'src/protobuf/mir_protobuf.proto'
294--- src/protobuf/mir_protobuf.proto 2016-12-08 04:08:06 +0000
295+++ src/protobuf/mir_protobuf.proto 2016-12-14 04:10:39 +0000
296@@ -241,6 +241,7 @@
297 optional bytes gamma_blue = 22;
298 optional uint32 gamma_supported = 23;
299 optional bytes edid = 24;
300+ optional string model = 25;
301 }
302
303 message Connection {
304
305=== modified file 'src/server/report/logging/display_configuration_report.cpp'
306--- src/server/report/logging/display_configuration_report.cpp 2016-11-30 03:00:24 +0000
307+++ src/server/report/logging/display_configuration_report.cpp 2016-12-14 04:10:39 +0000
308@@ -20,6 +20,7 @@
309 #include "mir/graphics/display_configuration.h"
310 #include "mir/output_type_names.h"
311 #include "mir/logging/logger.h"
312+#include "mir/graphics/edid.h"
313
314 #include <boost/exception/diagnostic_information.hpp>
315 #include <cmath>
316@@ -93,6 +94,22 @@
317 );
318 if (out.connected)
319 {
320+ using mir::graphics::Edid;
321+ if (out.edid.size() >= Edid::minimum_size)
322+ {
323+ auto edid = reinterpret_cast<Edid const*>(out.edid.data());
324+ Edid::MonitorName name;
325+ if (edid->get_monitor_name(name))
326+ logger->log(component, severity,
327+ "%sEDID monitor name: %s", indent, name);
328+ Edid::Manufacturer man;
329+ edid->get_manufacturer(man);
330+ logger->log(component, severity,
331+ "%sEDID manufacturer: %s", indent, man);
332+ logger->log(component, severity,
333+ "%sEDID product code: %hu", indent, edid->product_code());
334+ }
335+
336 int width_mm = out.physical_size_mm.width.as_int();
337 int height_mm = out.physical_size_mm.height.as_int();
338 float inches =
339
340=== modified file 'src/utils/out.c'
341--- src/utils/out.c 2016-12-08 04:08:06 +0000
342+++ src/utils/out.c 2016-12-14 04:10:39 +0000
343@@ -426,6 +426,10 @@
344
345 if (state == mir_output_connection_state_connected)
346 {
347+ char const* model = mir_output_get_model(out);
348+ if (model)
349+ printf(", \"%s\"", model);
350+
351 MirOutputMode const* current_mode =
352 mir_output_get_current_mode(out);
353 if (current_mode)
354
355=== modified file 'tests/acceptance-tests/test_new_display_configuration.cpp'
356--- tests/acceptance-tests/test_new_display_configuration.cpp 2016-12-08 04:08:06 +0000
357+++ tests/acceptance-tests/test_new_display_configuration.cpp 2016-12-14 04:10:39 +0000
358@@ -1085,6 +1085,90 @@
359 client.disconnect();
360 }
361
362+TEST_F(DisplayConfigurationTest, client_receives_model_string_from_edid)
363+{
364+ static unsigned char const edid[] =
365+ "\x00\xff\xff\xff\xff\xff\xff\x00\x10\xac\x46\xf0\x4c\x4a\x31\x41"
366+ "\x05\x19\x01\x04\xb5\x34\x20\x78\x3a\x1d\xf5\xae\x4f\x35\xb3\x25"
367+ "\x0d\x50\x54\xa5\x4b\x00\x81\x80\xa9\x40\xd1\x00\x71\x4f\x01\x01"
368+ "\x01\x01\x01\x01\x01\x01\x28\x3c\x80\xa0\x70\xb0\x23\x40\x30\x20"
369+ "\x36\x00\x06\x44\x21\x00\x00\x1a\x00\x00\x00\xff\x00\x59\x43\x4d"
370+ "\x30\x46\x35\x31\x52\x41\x31\x4a\x4c\x0a\x00\x00\x00\xfc\x00\x44"
371+ "\x45\x4c\x4c\x20\x55\x32\x34\x31\x33\x0a\x20\x20\x00\x00\x00\xfd"
372+ "\x00\x38\x4c\x1e\x51\x11\x00\x0a\x20\x20\x20\x20\x20\x20\x01\x42"
373+ "\x02\x03\x1d\xf1\x50\x90\x05\x04\x03\x02\x07\x16\x01\x1f\x12\x13"
374+ "\x14\x20\x15\x11\x06\x23\x09\x1f\x07\x83\x01\x00\x00\x02\x3a\x80"
375+ "\x18\x71\x38\x2d\x40\x58\x2c\x45\x00\x06\x44\x21\x00\x00\x1e\x01"
376+ "\x1d\x80\x18\x71\x1c\x16\x20\x58\x2c\x25\x00\x06\x44\x21\x00\x00"
377+ "\x9e\x01\x1d\x00\x72\x51\xd0\x1e\x20\x6e\x28\x55\x00\x06\x44\x21"
378+ "\x00\x00\x1e\x8c\x0a\xd0\x8a\x20\xe0\x2d\x10\x10\x3e\x96\x00\x06"
379+ "\x44\x21\x00\x00\x18\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00"
380+ "\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x09";
381+
382+ mtd::StubDisplayConfigurationOutput monitor{
383+ mg::DisplayConfigurationOutputId{48},
384+ {{{1920, 1200}, 60.0}},
385+ {mir_pixel_format_abgr_8888}};
386+ monitor.edid.assign(edid, edid+sizeof(edid)-1);
387+
388+ auto config = std::make_shared<mtd::StubDisplayConfig>(
389+ std::vector<mg::DisplayConfigurationOutput>{monitor});
390+
391+ apply_config_change_and_wait_for_propagation(config);
392+
393+ DisplayClient client{new_connection()};
394+ client.connect();
395+
396+ auto base_config = client.get_base_config();
397+ auto output = mir_display_config_get_output(base_config.get(), 0);
398+
399+ EXPECT_STREQ("DELL U2413", mir_output_get_model(output));
400+
401+ client.disconnect();
402+}
403+
404+TEST_F(DisplayConfigurationTest, client_receives_fallback_string_from_edid)
405+{
406+ static unsigned char const edid[] =
407+ "\x00\xff\xff\xff\xff\xff\xff\x00\x10\xac\x46\xf0\x4c\x4a\x31\x41"
408+ "\x05\x19\x01\x04\xb5\x34\x20\x78\x3a\x1d\xf5\xae\x4f\x35\xb3\x25"
409+ "\x0d\x50\x54\xa5\x4b\x00\x81\x80\xa9\x40\xd1\x00\x71\x4f\x01\x01"
410+ "\x01\x01\x01\x01\x01\x01\x28\x3c\x80\xa0\x70\xb0\x23\x40\x30\x20"
411+ "\x36\x00\x06\x44\x21\x00\x00\x1a\x00\x00\x00\xff\x00\x59\x43\x4d"
412+ "\x30\x46\x35\x31\x52\x41\x31\x4a\x4c\x0a\x00\x00\x00\x11\x00\x44"
413+ "\x45\x4c\x4c\x20\x55\x32\x34\x31\x33\x0a\x20\x20\x00\x00\x00\xfd"
414+ "\x00\x38\x4c\x1e\x51\x11\x00\x0a\x20\x20\x20\x20\x20\x20\x01\x42"
415+ "\x02\x03\x1d\xf1\x50\x90\x05\x04\x03\x02\x07\x16\x01\x1f\x12\x13"
416+ "\x14\x20\x15\x11\x06\x23\x09\x1f\x07\x83\x01\x00\x00\x02\x3a\x80"
417+ "\x18\x71\x38\x2d\x40\x58\x2c\x45\x00\x06\x44\x21\x00\x00\x1e\x01"
418+ "\x1d\x80\x18\x71\x1c\x16\x20\x58\x2c\x25\x00\x06\x44\x21\x00\x00"
419+ "\x9e\x01\x1d\x00\x72\x51\xd0\x1e\x20\x6e\x28\x55\x00\x06\x44\x21"
420+ "\x00\x00\x1e\x8c\x0a\xd0\x8a\x20\xe0\x2d\x10\x10\x3e\x96\x00\x06"
421+ "\x44\x21\x00\x00\x18\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00"
422+ "\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x09";
423+
424+ mtd::StubDisplayConfigurationOutput monitor{
425+ mg::DisplayConfigurationOutputId{2},
426+ {{{3210, 2800}, 60.0}},
427+ {mir_pixel_format_abgr_8888}};
428+ monitor.edid.assign(edid, edid+sizeof(edid)-1);
429+
430+ auto config = std::make_shared<mtd::StubDisplayConfig>(
431+ std::vector<mg::DisplayConfigurationOutput>{monitor});
432+
433+ apply_config_change_and_wait_for_propagation(config);
434+
435+ DisplayClient client{new_connection()};
436+ client.connect();
437+
438+ auto base_config = client.get_base_config();
439+ auto output = mir_display_config_get_output(base_config.get(), 0);
440+
441+ EXPECT_STREQ("DEL 61510", mir_output_get_model(output));
442+
443+ client.disconnect();
444+}
445+
446 namespace
447 {
448 MATCHER_P(IsSameModeAs, mode, "")
449
450=== modified file 'tests/unit-tests/CMakeLists.txt'
451--- tests/unit-tests/CMakeLists.txt 2016-11-15 23:48:01 +0000
452+++ tests/unit-tests/CMakeLists.txt 2016-12-14 04:10:39 +0000
453@@ -82,6 +82,7 @@
454 test_posix_rw_mutex.cpp
455 test_posix_timestamp.cpp
456 test_observer_multiplexer.cpp
457+ test_edid.cpp
458 )
459
460 CMAKE_DEPENDENT_OPTION(
461
462=== added file 'tests/unit-tests/test_edid.cpp'
463--- tests/unit-tests/test_edid.cpp 1970-01-01 00:00:00 +0000
464+++ tests/unit-tests/test_edid.cpp 2016-12-14 04:10:39 +0000
465@@ -0,0 +1,73 @@
466+/*
467+ * Copyright © 2016 Canonical Ltd.
468+ *
469+ * This program is free software: you can redistribute it and/or modify it
470+ * under the terms of the GNU General Public License version 3,
471+ * as published by the Free Software Foundation.
472+ *
473+ * This program is distributed in the hope that it will be useful,
474+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
475+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
476+ * GNU General Public License for more details.
477+ *
478+ * You should have received a copy of the GNU General Public License
479+ * along with this program. If not, see <http://www.gnu.org/licenses/>.
480+ *
481+ * Author: Daniel van Vugt <daniel.van.vugt@canonical.com>
482+ */
483+
484+#include "mir/graphics/edid.h"
485+#include <gtest/gtest.h>
486+
487+using mir::graphics::Edid;
488+
489+namespace
490+{
491+unsigned char const dell_u2413_edid[] =
492+ "\x00\xff\xff\xff\xff\xff\xff\x00\x10\xac\x46\xf0\x4c\x4a\x31\x41"
493+ "\x05\x19\x01\x04\xb5\x34\x20\x78\x3a\x1d\xf5\xae\x4f\x35\xb3\x25"
494+ "\x0d\x50\x54\xa5\x4b\x00\x81\x80\xa9\x40\xd1\x00\x71\x4f\x01\x01"
495+ "\x01\x01\x01\x01\x01\x01\x28\x3c\x80\xa0\x70\xb0\x23\x40\x30\x20"
496+ "\x36\x00\x06\x44\x21\x00\x00\x1a\x00\x00\x00\xff\x00\x59\x43\x4d"
497+ "\x30\x46\x35\x31\x52\x41\x31\x4a\x4c\x0a\x00\x00\x00\xfc\x00\x44"
498+ "\x45\x4c\x4c\x20\x55\x32\x34\x31\x33\x0a\x20\x20\x00\x00\x00\xfd"
499+ "\x00\x38\x4c\x1e\x51\x11\x00\x0a\x20\x20\x20\x20\x20\x20\x01\x42"
500+ "\x02\x03\x1d\xf1\x50\x90\x05\x04\x03\x02\x07\x16\x01\x1f\x12\x13"
501+ "\x14\x20\x15\x11\x06\x23\x09\x1f\x07\x83\x01\x00\x00\x02\x3a\x80"
502+ "\x18\x71\x38\x2d\x40\x58\x2c\x45\x00\x06\x44\x21\x00\x00\x1e\x01"
503+ "\x1d\x80\x18\x71\x1c\x16\x20\x58\x2c\x25\x00\x06\x44\x21\x00\x00"
504+ "\x9e\x01\x1d\x00\x72\x51\xd0\x1e\x20\x6e\x28\x55\x00\x06\x44\x21"
505+ "\x00\x00\x1e\x8c\x0a\xd0\x8a\x20\xe0\x2d\x10\x10\x3e\x96\x00\x06"
506+ "\x44\x21\x00\x00\x18\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00"
507+ "\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x09";
508+} // namespace
509+
510+TEST(EDID, has_correct_size)
511+{
512+ EXPECT_EQ(128u, sizeof(Edid));
513+ EXPECT_EQ(128u, Edid::minimum_size);
514+}
515+
516+TEST(EDID, can_get_name)
517+{
518+ auto edid = reinterpret_cast<Edid const*>(dell_u2413_edid);
519+ Edid::MonitorName name;
520+ int len = edid->get_monitor_name(name);
521+ EXPECT_EQ(10, len);
522+ EXPECT_STREQ("DELL U2413", name);
523+}
524+
525+TEST(EDID, can_get_manufacturer)
526+{
527+ auto edid = reinterpret_cast<Edid const*>(dell_u2413_edid);
528+ Edid::Manufacturer man;
529+ int len = edid->get_manufacturer(man);
530+ EXPECT_EQ(3, len);
531+ EXPECT_STREQ("DEL", man);
532+}
533+
534+TEST(EDID, can_get_product_code)
535+{
536+ auto edid = reinterpret_cast<Edid const*>(dell_u2413_edid);
537+ EXPECT_EQ(61510u, edid->product_code());
538+}

Subscribers

People subscribed via source and target branches