-
Notifications
You must be signed in to change notification settings - Fork 242
feat/Add active hostname to device display #896
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
base: dev
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR adds the ability to display the active hostname on the device's home screen, helping users distinguish between multiple JetKVM devices at a glance.
Key Changes:
- Added a new label widget to display the hostname on the home screen
- Integrated hostname retrieval from the network manager
Added a new font style (LabelFontBook24) to support varied text sizing
Reviewed Changes
Copilot reviewed 13 out of 17 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| internal/native/eez/src/ui/styles.c | Made layout adjustments to flex styles |
| internal/native/eez/src/ui/screens.h | Added home_info_hostname object declaration |
| internal/native/eez/src/ui/screens.c | Created hostname label widget and adjusted spacing/layout across multiple screens |
| internal/native/eez/jetkvm.eez-project-ui-state | Removed UI state file (project-specific configuration) |
| display.go | Added call to update hostname label from network manager |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
55c88ca to
ffca856
Compare
adamshiervani
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The screen is too crowded with IPv4, IPv6, MAC and hostname. Let's just replace the MAC address with the hostname if we have an IPv4 IP. Once the user has an IP, the MAC address isn't that useful to have on the device. If they really need it, they can copy it from the network settings.
Done, also was able to restore the 30pt font on the IPv4 |
d6ee845 to
13d348c
Compare
c082bb4 to
ca63198
Compare
|
Did a bunch of work on the JetKVM display (EEZ Studio stuff)
The way things are working correctly we want the following things in objects in EEZ Studio:
|
That's very odd, as this is what I've been fighting with all along to get consistent behavior. Can you verify that your jetkvm.eez-project file matches what is currently checked in? (I did a bunch of force-pushes right at the end to get this over the line). Could you try running EEZ Studio and clicking the Build and seeing if it tries to change any of the "output" files in internal/native/eez/src/ui directory? Just checking back here:
Please note any pages above that you want me to enable what's hidden and then we'll have a lot more scrolling ;)
|
ca63198 to
0e1c49c
Compare
|
(oops, I accidentally edited your comment instead of replying, sorry @adamshiervani) |
Also regularized up the layouts
Also cleaned up all the scrolling and click/gesture handling so all the menus work correctly.
51e0c9e to
f234807
Compare
| nativeInstance.UpdateLabelAndChangeVisibility("golang_version", version.GoVersion) | ||
|
|
||
| // nativeInstance.UpdateLabelAndChangeVisibility("boot_screen_device_id", GetDeviceID()) | ||
| nativeInstance.UpdateLabelAndChangeVisibility("device_id", GetDeviceID()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we use UISetVar here too and remove UpdateLabelAndChangeVisibility completely?
EEZ has the ability to change visibility based on a variable, but I'm not sure whether we want to complicate the EEZ code or the Golang code.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think absolutely that would be much simpler! I just didn't know if you folks would mind that big a change. I'll do it (my) Friday night.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can do it in a separate PR instead. Let's keep this one PR from getting too big :-)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure thing.
Any thoughts about Adam's comments? Do we need to reenable and of those other screens?
Closes #617
Summary
Folks with more than one JetKVM want to be able to distinguish them at a glace. The current device display Home screen has enough space to display a name, and with recent networking changes, displaying the user-defined (or defaulted) hostname will help.
UI Changes
Checklist
Closes #<issue-number>)[ ] Tricky parts are commented in codeN/A[ ] Backward compatible with existing device firmware (SeeN/ADEVELOPMENT.mdfor details)