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

ui: add button labels that tell the user which button does what #56

Merged
merged 4 commits into from
Mar 13, 2024

Conversation

hnez
Copy link
Member

@hnez hnez commented Jan 22, 2024

This adds a little legend on the right of the screen (where the two buttons are) that tells the user what which button does what:

dig_out iobus iobus_health locator overtemperature power power_fail reboot screensaver system uart update_available update_installing usb usb_overload reboot_slot

TODO before merging:

  • Add comments to the drawing routine that make it easier to read
  • Add comments to the screen rotation trickery
  • Add legends to alert screens as well

src/ui/display.rs Show resolved Hide resolved
src/ui/display.rs Outdated Show resolved Hide resolved
src/ui/screens/system.rs Show resolved Hide resolved
src/ui/screens/system.rs Outdated Show resolved Hide resolved
src/ui/widgets.rs Outdated Show resolved Hide resolved
src/ui/widgets.rs Show resolved Hide resolved
@hnez hnez marked this pull request as ready for review January 26, 2024 14:03
@hnez hnez requested a review from KarlK90 January 26, 2024 14:04
@hnez
Copy link
Member Author

hnez commented Jan 26, 2024

I've now gone through all screens and added legends.
With that I am ready for review.

@SmithChart SmithChart added this to the second stable release milestone Mar 1, 2024
@SmithChart
Copy link
Member

SmithChart commented Mar 4, 2024

First: I find this really helpful. It feels like the first time that I really know what button does which action. Or at least this way a user can learn to use them right. (Maybe we can add caps with different colors to the buttons, to make it even easier to remember?)

@fiurin and I had a look at this change from a user-perspective. One thing we kind of stumbled across is the label on the lower button. This button can either be used with a short- or long press. And does different things depending on the type of press.
For screens with more than one option the toggle or select labels are not clear. We would vote to use Action here instead. This is intended to be ominous and vague, so the user can assume that both long- and short press are available here. This applies to the "USB Host", "Digital out", "DUT UART" and "System screens".

This would look like this:
content

I would vote to leave all other screens as they are in this PR.

@hnez
Copy link
Member Author

hnez commented Mar 5, 2024

I've rebased the PR on top of recent main (it should go in after #59 though, so we see the CI succeed before merging) and changed the labels in the mentioned screens from what they were there to "Action".

@SmithChart
Copy link
Member

I've rebased the PR on top of recent main (it should go in after #59 though, so we see the CI succeed before merging) and changed the labels in the mentioned screens from what they were there to "Action".

Thanks! I tried the recent changes on my device. Looks good!

hnez added 4 commits March 5, 2024 10:14
Currently draw_border does its own locking of the display,
but most callees also lock the screen to draw UI elements or
will do so soon when button legends are introduced.

Make draw_border use the already locked display instead of doing
its own locking.

This is mostly an esthetic change, as the framebuffer backend is
not at all concerned with the locking we do here and performance-wise
it should not make a difference.

Signed-off-by: Leonard Göhrs <l.goehrs@pengutronix.de>
The system screen contains information like this:

    SoC:    30C
    Uplink: 1000MBit/s
    DUT:    Down
    IP:     123.123.123.123

The IP line would overflow into the button legends that are about to be,
added so shorten the "Uplink:" label to "UL:" and move everything left:

    SoC: 30C
    UL:  1000MBit/s
    DUT: Down
    IP:  123.123.123.123

Signed-off-by: Leonard Göhrs <l.goehrs@pengutronix.de>
The legend is drawn at the right of the screen using rotated text.
The embedded_graphics library does not have support for rotating
elements like text, so we instead provide it with a _view_ of the
screen that is rotated and onto which it can render its text as
normal.

Signed-off-by: Leonard Göhrs <l.goehrs@pengutronix.de>
… status

For these two screens it is rather easy to make the legend dynamic.
For other screens it is a bit more complicated, so only do these two first.

Signed-off-by: Leonard Göhrs <l.goehrs@pengutronix.de>
Copy link
Member

@KarlK90 KarlK90 left a comment

Choose a reason for hiding this comment

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

The changes themselves look good to me. My main concern is that we are starting to reinvent a proper GUI toolkit here - which is already a solved problem. It would be nice to investigate if LVGL which has offical Rust bindings and is MIT licensed handles what we need.

@hnez
Copy link
Member Author

hnez commented Mar 13, 2024

Using an actual graphics toolkit, like LVGL for instance, would be great I think, because it could make the UI more modern (and maybe make working on the UI a bit easier).
I will propose an evaluation of GUI options for the TAC and other products with tiny displays and reduced input options as a techweek project once that comes around.

@hnez hnez merged commit 998b73c into linux-automation:main Mar 13, 2024
10 checks passed
@hnez hnez deleted the button-labels branch March 13, 2024 06:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants