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.

Sign up for GitHub

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

Jump to bottom

fix(ui) - LVGL conversion for telemetry. #3070

Merged
merged 4 commits into from
Feb 3, 2023
Merged

fix(ui) - LVGL conversion for telemetry. #3070

merged 4 commits into from
Feb 3, 2023

Conversation

philmoz
Copy link
Collaborator

@philmoz philmoz commented Jan 21, 2023

Conversion to LVGL flex layout for telemetry pages.

For portrait layout, long strings like GPS are displayed in small font in order to fit.

Screen Shot 2023-01-21 at 12 19 23 pm

Screen Shot 2023-01-21 at 12 19 53 pm

Screen Shot 2023-01-21 at 12 20 06 pm

Screen Shot 2023-01-21 at 12 20 28 pm

Screen Shot 2023-01-21 at 12 21 13 pm

Screen Shot 2023-01-21 at 12 23 01 pm

Screen Shot 2023-01-21 at 12 24 07 pm

@philmoz philmoz mentioned this pull request Jan 21, 2023
Closed
12 tasks
@eshifri
Copy link
Contributor

eshifri commented Jan 21, 2023

AFAIK for everything but FrSky alarms are not based on RSSI but on some other sensors (e.g. for FlySky it is Sgnl), so maybe different name would be better.

@pfeerick pfeerick added the color Related generally to color LCD radios label Jan 23, 2023
@pfeerick pfeerick self-assigned this Jan 26, 2023
@pfeerick
Copy link
Member

AFAIK for everything but FrSky alarms are not based on RSSI but on some other sensors (e.g. for FlySky it is Sgnl), so maybe different name would be better.

True, but it's not a static label - it changes with the protocol - getRssiLabel()

@JimB40
Copy link
Collaborator

JimB40 commented Jan 26, 2023

  1. Do we need sensor ID on the list?
  2. As I can see sensor lost is indicated by text color change to WARNING.
    How incoming telemetry data trasmission is indicated?

@philmoz
Copy link
Collaborator Author

philmoz commented Jan 26, 2023

How incoming telemetry data trasmission is indicated?

Do you mean the blue dot to the left of the value?
Screen Shot 2023-01-26 at 5 13 33 pm

@eshifri
Copy link
Contributor

eshifri commented Jan 26, 2023

IMO sensor ID is VERY nice to have for development/debugging. It may be convenient for flying you have several similar sensors.

@JimB40
Copy link
Collaborator

JimB40 commented Jan 26, 2023 •
edited
Loading

Do you mean the blue dot to the left of the value?

Ok so it's like before. Just I'd v-center and h-center in column between sensor number and sensor name.

@JimB40
Copy link
Collaborator

JimB40 commented Jan 26, 2023 •
edited
Loading

IMO sensor ID is VERY nice to have for development/debugging. It may be convenient for flying you have several similar sensors.

Hmm so we can have "dev-mode showing that". Without ID column GPS coords will fit nicely in vertical layout
What is purpose to see in overview list that AccX & A3 and all the rest is 224?
Is it useful anyhow for user (except developer) like this ID is used elswhere in OS as input data?

@eshifri
Copy link
Contributor

eshifri commented Jan 26, 2023

I think that dev-mode might be sufficient. Especially if it can be configurable rather than compile-time option.

@JimB40
Copy link
Collaborator

JimB40 commented Jan 26, 2023 •
edited
Loading

@philmoz one more thing
Sensors list is most used feature in this tab. RC Link alert (RSSI) and Vario is set & forget. So I'd switch group order to (from top to bottom):

  1. Sensors List
  2. RC Link Alerts
  3. Variometer

@pfeerick
Copy link
Member

pfeerick commented Jan 26, 2023 •
edited
Loading

Hmm so we can have "dev-mode showing that".

Not so much a "dev" mode, as end users will need that... more perhaps another checkbox like the "Ignore Instances" one but this one would be "Show Sensor ID" or similar... if we can update the table on the fly / rebuild the page if necessary (i.e. not need the user to leave the page to see the change). Otherwise that option would need to go somewhere else. I suspect worst case GPS position still may not fit - so two lines may be better for that.

@JimB40
Copy link
Collaborator

JimB40 commented Jan 27, 2023

I bet 5% users will need it but if so we already have layout template in Mixes
Screenshot 2023-01-27 at 15 12 33
So place "Show Sensor ID" below sensors list and display detailed data with TINSIZE right aligned above sensor value.
If needed switch can be "Show Sensor Details" and data can be Type / ID / Instance (if set)

@pfeerick pfeerick added this to the 2.9 milestone Jan 28, 2023
@pfeerick pfeerick merged commit 41907f5 into EdgeTX:main Feb 3, 2023
@pfeerick
Copy link
Member

pfeerick commented Feb 3, 2023

@philmoz Can you take the below onboard as a future enhancement to this PR? Another thought might be to double-line the GPS co-oords so that the font size stays the same.

we already have layout template in Mixes Screenshot 2023-01-27 at 15 12 33 So place "Show Sensor ID" below sensors list and display detailed data with TINYSIZE right aligned above sensor value. If needed, switch can be "Show Sensor Details" and data can be Type / ID / Instance (if set)

@eshifri eshifri mentioned this pull request Feb 5, 2023
Closed
@ParkerEde
Copy link
Contributor

@philmoz
Even if this has already been merged here, could we take this opportunity to ensure that when you press the "Start sensor search" button, the focus remains on this button, so that you can also stop the sensor search again straight away?
It is also confusing that when you delete a sensor, the focus jumps up in the list. It could also jump to the previous entry instead, then you don't have to scroll unnecessarily while you maintain / clean up your sensor data.

@ParkerEde
Copy link
Contributor

Addendum: The Start Sensor Search button loses focus as soon as a new sensor is found.

philmoz pushed a commit to philmoz/edgetx that referenced this pull request Feb 13, 2023
Merge branch 'main' into companion-joystick
a241536 
* main: (30 commits)
  feat(cpn): Radiomaster Boxer support (EdgeTX#2910)
  chore: Move __global_locale to FLASH (frees some RAM!) (EdgeTX#3169)
  chore: Remove obsolete language defines, single source for telemetry sensor names (EdgeTX#3119)
  feat(lua): Port of NodeMCU Lua53 read-only tables (EdgeTX#2994)
  chore: New module/serial API (EdgeTX#3055)
  chore: Updated SE translations 🇸🇪 (EdgeTX#3148)
  Update README.md
  fix(color): Duplicate selected theme, update color list (EdgeTX#3122)
  fix: Regenerate yaml, carryTrim => trimSource cleanup (EdgeTX#3121)
  chore(color): LVGLify custom mixer scripts page (EdgeTX#3071)
  fix(translation): String casting for CZ/IT/FR Fixes EdgeTX#3145
  fix(cpn): Save "set main view" number for special/global function (EdgeTX#3132)
  chore(color): LVGLify statistics and debug screens (EdgeTX#3072)
  chore(color): LVGLify and enhance model -> telemetry page (EdgeTX#3070)
  chore: Updates to Danish translations 🇩🇰 (EdgeTX#3128)
  chore: CN/TW translations for theme save and delete strings (EdgeTX#3129)
  chore: fix STM32 HAL headers
  fix(color): Overlap in logical switch monitor footer (EdgeTX#3137)
  fix(doc): LUA doc link to units.md
  feat(color): Change global variables layout to better match other pages (EdgeTX#3116)
  ...
@JimB40 JimB40 mentioned this pull request Feb 21, 2023
Open
23 tasks
@philmoz philmoz deleted the telemetry-layout branch September 20, 2023 02:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@pfeerick pfeerick

Labels
color Related generally to color LCD radios
Projects
None yet
Milestone
2.9
Development

Successfully merging this pull request may close these issues.
5 participants
@philmoz @eshifri @pfeerick @JimB40 @ParkerEde