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

feat(bw128): display PPM value in channel monitor #5781

Merged
merged 2 commits into from
Jan 19, 2025

Conversation

philmoz
Copy link
Collaborator

@philmoz philmoz commented Jan 14, 2025

screenshot_gx12_25-01-14_18-29-33

@philmoz philmoz added enhancement ✨ New feature or request UX-UI Related to user experience (UX) or user interface (UI) behaviour B&W Related generally to black and white LCD radios labels Jan 14, 2025
@philmoz philmoz added this to the 2.11 milestone Jan 14, 2025
@pfeerick pfeerick changed the title feat(B&W): display PPM value in channel monitor on 128x64 B&W radios. feat(bw128): display PPM value in channel monitor Jan 14, 2025
@ajjjjjjjj
Copy link
Contributor

Great addition!
I would also suggest this for better alignment for channels 10+:

constexpr coord_t CHANNEL_VALUE_OFFSET = CHANNEL_NAME_OFFSET + 43;
constexpr coord_t CHANNEL_GAUGE_OFFSET = CHANNEL_VALUE_OFFSET - 1;

@philmoz
Copy link
Collaborator Author

philmoz commented Jan 15, 2025

Need to rethink this. The space on the right is also used for the override and inverted indicators ('OVR' and 'INV').

@ajjjjjjjj
Copy link
Contributor

Maybe | BLINK for "INV" and "OVR"?

@pfeerick
Copy link
Member

Probably should be thinking about if this change really makes sense to make ... as currently the channel value column follows the PPM Units setting... (which probably needs to be renamed, but that is a different topic)... and thus allows the user the choose what is shown in most places in the UI for the channel value... -100 to 100, -100.0 to 100 or 988-2012...

This change not only does not honour that configuration (i.e. why does it show both if I have configured that I want 0.0?), but also has that side effect of messing up the UI as mentioned (OVR/INV)...

i.e. before
image

after
image

I just wonder if heading down this path this is going to make the UI more cluttered or confusing by adding animated elements to make up for the limited space...

@philmoz
Copy link
Collaborator Author

philmoz commented Jan 15, 2025

Blinking the OVR and INV states:
https://github.com/user-attachments/assets/441251f0-629c-429b-93cc-56d5439a0416

If PPM Units is set to 'us' then the values swap sides (PPM on left, % on right). This is similar to what the color channel monitor does:
https://github.com/user-attachments/assets/ae65a80e-51c2-4341-99d1-41060d62d131

@philmoz
Copy link
Collaborator Author

philmoz commented Jan 16, 2025

I've updated the PR to alternate the right side view between the value, OVR and INV indicators.
This also means both OVR and INV can be shown if set (previously OVR took precedence).

Also adjusted the layout to stop the percentage value overlapping the channel name when units is set to show the decimal point.

Units without decimal:
screenshot_gx12_25-01-16_11-26-55

Units with decimal:

screenshot_gx12_25-01-16_11-27-18

PPM units:
screenshot_gx12_25-01-16_11-27-49

@philmoz philmoz marked this pull request as ready for review January 16, 2025 01:17
@philmoz philmoz force-pushed the philmoz/bw-chan-mon-ppm branch from 5c550dd to fdfc9bd Compare January 17, 2025 20:11
@pfeerick pfeerick merged commit e41680b into main Jan 19, 2025
49 checks passed
@pfeerick pfeerick deleted the philmoz/bw-chan-mon-ppm branch January 19, 2025 01:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
B&W Related generally to black and white LCD radios enhancement ✨ New feature or request UX-UI Related to user experience (UX) or user interface (UI) behaviour
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants