feat(display.go): Add display brightness settings #17

Nevexo · 2025-01-03T11:25:08Z

This PR introduces:

Altering the display brightness.
Dimming the display after a period of activity.
Turn off the display, after even longer.
Waking the display on system event (connection, HDMI/USB/Net up/down, touch event).
All of the above can be set in the Web UI

Settings UI for these changes

Will merge conflict with #28 - so one of them will need rebasing.

Just want to try the feature? The latest version is available in my tree, jetkvm-next: https://github.com/Nevexo/jetkvm-kvm/releases/tag/next-6

Implements setDisplayBrightness(brightness int) which allows setting the backlight brightness on JetKVM's hardware. Needs to be implemented into the RPC, config and frontend.

WIP, dims the display to 50% of the BacklightMaxBrightness after BacklightDimAfterMS expires. Turns the display off after BacklightOffAfterMS

Nevexo · 2025-01-03T22:10:57Z

The current branch automatically dims, and then switches off, the display when configured.

However, the display doesn't re-activate when touched, so the touchscreen menu is inaccessible once the display goes into the "off" state. This needs some extra work with jetkvm_native, either backlight control needs to move into jetkvm_native, or native needs to send the application a message when the display is touched, so it can call wakeDisplay()

The recent changes also introduce new RPC commands to access and update the backlight config (maxbrightness, dimafter, onafter), and adds it to the settings pane of the frontend, but currently doesn't change the max brightness value due to a type miss-match between the frontend code and the jsonrpc handler.

Very much WIP, and can't be merged as it breaks the touchscreen feature at the moment.

Nevexo · 2025-01-04T09:52:54Z

Current plan to deal with the touchscreen, at least until we have access to jetkvm_native, is to watch /dev/input/event for touch events and call wakeDisplay, as found by Tutman.

Nevexo · 2025-01-04T22:25:08Z

Display now wakes when the touchscreen is touched, either from dimmed or entirely off thanks to @tutman96's suggestion of reading /dev/input/events1 - needs a better solution eventually but this will do for now.

WIll work on RPC and frontend.

If the application had turned off the display before exiting, it wouldn't be brought on when the application restarted without a device reboot.

display.go

tutman96 · 2025-01-05T00:08:33Z

display.go

+
+	go func() {
+		// Start display auto-sleeping ticker
+		ticker := time.NewTicker(1 * time.Second)


Since you are already using a ticker, you might want to consider using https://pkg.go.dev/time#Ticker.Reset to just Reset it back to the DisplayDimAfterMs time whenever there is activity. Would be a bit more efficient and would mean you wouldn't need to have this no-op loop running once every second.

Neat, I'll do that.

@tutman96 Okay, pushed this - please take a look when you get chance, I'm not super happy with having two methods that are called when the tickers fire (tick_displayDim and tick_displayOff) as they practically do the same job. I wonder if it'd be possible to keep the tickers in an array and figure out which one of them fired, to have one handler for them?

display.go

@eric

Noticed by @eric jetkvm#17 (comment)

Set the DisplayMaxBrightness to the default brightness used out-of-the-box by JetKVM. Also sets the dim/timeout to 2 minutes and 30 mintes respectively.

As suggested by tutman in jetkvm#17, use tickers set to the duration of dim/off to avoid a loop running every second. The tickers are reset to the dim/off times whenever wakeDisplay() is called.

Changed Dim & Off values to seconds instead of milliseconds, there's no need for it to be that precise.

Adds the force boolean to wakedisplay() which allows skipping the backlightState == 0 check, this means you can force a ticker reset, even if the display is currently in the "full bright" state

Nevexo · 2025-01-27T21:20:24Z

The latest version of this is now available in jetkvm-next https://github.com/Nevexo/jetkvm-kvm/releases/tag/next-6

lwbt · 2025-01-27T21:53:45Z

The latest version of this is now available in jetkvm-next https://github.com/Nevexo/jetkvm-kvm/releases/tag/next-6

I this supposed to work like your next-5 release or do I need something else? Mine would not complete startup of the application and started to reboot after a while. I switched back to next-5.

Nevexo · 2025-01-27T21:57:17Z

I this supposed to work like your next-5 release or do I need something else

The config schema was updated, so that might be causing it to fall over on startup, try running jetkvm_app from /userdata/jetkvm/bin on it's own (you'll need to killall jetkvm_app first) and see what the output is.

Otherwise, edit /userdata/kvm_config.json and change DisplayDimAfterMs and DisplayOffAfterMs to DisplayDimAfterSec and DisplayOffAfterSec respectively. If you're in the Discord it's probably easier to follow up in there, or open an issue on my repo if you can't get it going.

lwbt · 2025-01-27T22:22:39Z

I get this error:

root@JetKVM:~$  ./jetkvm_app_next_6
Starting mDNS server
2025/01/27 22:14:48 USB state changed from unknown to configured
display not inited, skipping updates
2025/01/27 22:14:48 No active RPC session, skipping update state update
2025/01/27 22:14:48 Server listening on /var/run/jetkvm_ctrl.sock
2025/01/27 22:14:48 Server listening on /var/run/jetkvm_video.sock
Syncing system time
2025/01/27 22:14:48 Reconciling plugin tailscale running , should be running 0.1.0
2025/01/27 22:14:48 Started plugin tailscale version 0.1.0
2025/01/27 22:14:48 Starting process: ./jetkvm-plugin-tailscale
Binary started with PID: 292
2025/01/27 22:14:48 Accepted plugin rpc connection from @
2025/01/27 22:14:48 Plugin rpc server started. Getting supported methods...
panic: assignment to entry in nil map

goroutine 46 [running]:
kvm/internal/jsonrpc.(*JSONRPCRouter).Request(0x1ba4c60, {0x8c36c1, 0x19}, 0x0, {0x7ba600, 0x1ba2c30})
        /home/cfleming/src/jetkvm/kvm/internal/jsonrpc/router.go:53 +0x1d4
kvm/internal/plugin.(*PluginRpcServer).handleRpcStatus(0x1ba4b70, {0x11708c4, 0x1ba4c90}, 0x1ba4c60)
        /home/cfleming/src/jetkvm/kvm/internal/plugin/rpc.go:137 +0x110
created by kvm/internal/plugin.(*PluginRpcServer).handleConnection in goroutine 45
        /home/cfleming/src/jetkvm/kvm/internal/plugin/rpc.go:104 +0x150

Otherwise, edit /userdata/kvm_config.json and change DisplayDimAfterMs and DisplayOffAfterMs to DisplayDimAfterSec and DisplayOffAfterSec respectively.

My File does not include any of this as far as I can see.

root@JetKVM:~$ cat /userdata/kvm_config.json:

{
  "cloud_url": "https://api.jetkvm.com",
  "cloud_token": "",
  "google_identity": "",
  "jiggler_enabled": false,
  "auto_update_enabled": true,
  "include_pre_release": true,
  "hashed_password": "",
  "localAuthMode": "noPassword",
  "wake_on_lan_devices": null
}

Nevexo · 2025-01-27T22:31:55Z

That's something strange going on with the plugin system, I'll have a poke around to see what's happened there, I assume I've buggered up a merge conflict.

Nevexo · 2025-01-27T22:47:19Z

Yeah something's happening with the plugin module. You can delete the plugin from /userdata/jetkvm/plugins, but then obviously you won't have the tail scale plugin anymore, so that's not a great solution.

I'll look into it more tomorrow, but since it's not an issue with the display feature, I'll track it here: Nevexo#1 - I'll ping you over there once I have a fix.

joshuasing

Thanks for working on this :D
I have added some comments on things that stood out to me whilst reading the diffs.

joshuasing · 2025-01-28T06:06:15Z

display.go

+	TOUCHSCREEN_DEVICE      string = "/dev/input/event1"
+	BACKLIGHT_CONTROL_CLASS string = "/sys/class/backlight/backlight/brightness"


Effective Go and the Go stdlib usually prefers lowerCamelCase/UpperCamelCase (depending on whether exported) for constant names. E.g., per https://go.dev/wiki/CodeReviewComments:

See https://go.dev/doc/effective_go#mixed-caps. This applies even when it breaks conventions in other languages. For example an unexported constant is maxLength not MaxLength or MAX_LENGTH.

Thanks! Updated.

joshuasing · 2025-01-28T06:26:38Z

display.go

+var dim_ticker *time.Ticker
+var off_ticker *time.Ticker


I think these variables (and the ones above) could be merged into blocks like this:

Suggested change

var dim_ticker *time.Ticker

var off_ticker *time.Ticker

var (

dim_ticker *time.Ticker

off_ticker *time.Ticker

)

Also - Effective Go and the Go stdlib usually prefers lowerCamelCase/UpperCamelCase (depending on whether exported) for names. E.g., per https://go.dev/wiki/CodeReviewComments:

See https://go.dev/doc/effective_go#mixed-caps. This applies even when it breaks conventions in other languages. For example an unexported constant is maxLength not MaxLength or MAX_LENGTH.

Some of the function names below also contain underscores, which is not best pratice.

Thanks, shifted those into one declaration and also renamed them to dimTicker and offTicker respectively.

Thank you for your review!

@joshuasing

As part of @joshuasing's review on jetkvm#17, updated variables & constants to match the Go best practices. Signed-off-by: Cameron Fleming <[email protected]>

Signed-off-by: Cameron Fleming <[email protected]>

@joshuasing

As part of @joshuasing's review on jetkvm#17, updated variables & constants to match the Go best practices. Signed-off-by: Cameron Fleming <[email protected]>

…sabled If max_brightness is zero, then there's no point in trying to dim it (or turn it off...)

The tickers only need to be reset, if they're disabled, they won't have been started.

joshuasing · 2025-01-30T14:16:52Z

display.go

+// tick_displayDim() is called when when dim ticker expires, it simply reduces the brightness
+// of the display by half of the max brightness.
+func tick_displayDim() {
+	err := setDisplayBrightness(config.DisplayMaxBrightness / 2)
+	if err != nil {
+		fmt.Printf("display: failed to dim display: %s\n", err)
+	}
+
+	dimTicker.Stop()
+
+	backlightState = 1
+}
+
+// tick_displayOff() is called when the off ticker expires, it turns off the display
+// by setting the brightness to zero.
+func tick_displayOff() {
+	err := setDisplayBrightness(0)
+	if err != nil {
+		fmt.Printf("display: failed to turn off display: %s\n", err)
+	}
+
+	offTicker.Stop()
+
+	backlightState = 2
+}


These function names contain underscores (which as mentioned in my other review, Effective Go and Go stdlib recommend using lowerCamelCase/UpperCamelCase - https://go.dev/doc/effective_go#mixed-caps) - Also, I think it is very uncommon to put () after the function name in doc comments (see https://go.dev/doc/comment):

(this suggestion does not update uses of the functions)

Suggested change

// tick_displayDim() is called when when dim ticker expires, it simply reduces the brightness

// of the display by half of the max brightness.

func tick_displayDim() {

err := setDisplayBrightness(config.DisplayMaxBrightness / 2)

if err != nil {

fmt.Printf("display: failed to dim display: %s\n", err)

}

dimTicker.Stop()

backlightState = 1

}

// tick_displayOff() is called when the off ticker expires, it turns off the display

// by setting the brightness to zero.

func tick_displayOff() {

err := setDisplayBrightness(0)

if err != nil {

fmt.Printf("display: failed to turn off display: %s\n", err)

}

offTicker.Stop()

backlightState = 2

}

// tickDisplayDim is called when when dim ticker expires, it simply reduces the brightness

// of the display by half of the max brightness.

func tickDisplayDim() {

err := setDisplayBrightness(config.DisplayMaxBrightness / 2)

if err != nil {

fmt.Printf("display: failed to dim display: %s\n", err)

}

dimTicker.Stop()

backlightState = 1

}

// tickDisplayOff is called when the off ticker expires, it turns off the display

// by setting the brightness to zero.

func tickDisplayOff() {

err := setDisplayBrightness(0)

if err != nil {

fmt.Printf("display: failed to turn off display: %s\n", err)

}

offTicker.Stop()

backlightState = 2

}

Thanks! I'll update these afterwards.

As suggested by tutman in jetkvm#17, use tickers set to the duration of dim/off to avoid a loop running every second. The tickers are reset to the dim/off times whenever wakeDisplay() is called.

@joshuasing

As part of @joshuasing's review on jetkvm#17, updated variables & constants to match the Go best practices. Signed-off-by: Cameron Fleming <[email protected]>

As suggested by tutman in jetkvm#17, use tickers set to the duration of dim/off to avoid a loop running every second. The tickers are reset to the dim/off times whenever wakeDisplay() is called.

@joshuasing

As part of @joshuasing's review on jetkvm#17, updated variables & constants to match the Go best practices. Signed-off-by: Cameron Fleming <[email protected]>

CLAassistant · 2025-02-13T13:30:06Z

All committers have signed the CLA.

Mauker1 · 2025-02-17T08:52:19Z

Question: Would you also add functionality for scheduled on/off times? It would be nice to switch the display at certain times.

Nevexo · 2025-02-17T08:53:43Z

Potentially. Probably not in this PR? Might be worth getting this one merged in first and open that as a new one afterwards.

chemhack · 2025-02-17T09:28:53Z

@Nevexo if you'd like to merge this one, just resolve the merge conflict then rebase WIP flag. we can do the touch input event from jetkvm_native in a separate pr.

Nevexo · 2025-02-17T09:34:32Z

@Nevexo if you'd like to merge this one, just resolve the merge conflict then rebase WIP flag. we can do the touch input event from jetkvm_native in a separate pr.

Conflict resolved, I don't have write access to merge it in but ready when you are 👍

Mauker1 · 2025-02-17T09:39:52Z

Potentially. Probably not in this PR? Might be worth getting this one merged in first and open that as a new one afterwards.

Sweet! Thanks for the quick reply :) And thank you for this PR too!

Nevexo marked this pull request as draft January 3, 2025 11:25

feat(display.go): impl setDisplayBrightness()

3271a17

Implements setDisplayBrightness(brightness int) which allows setting the backlight brightness on JetKVM's hardware. Needs to be implemented into the RPC, config and frontend.

Nevexo force-pushed the nevexo/display-brightness branch from aba3a02 to 3271a17 Compare January 3, 2025 11:42

Nevexo added 3 commits January 3, 2025 22:05

feat(config): add backlight control settings

4fd8b1e

feat(display): add automatic dimming & switch off to display

cd7258e

WIP, dims the display to 50% of the BacklightMaxBrightness after BacklightDimAfterMS expires. Turns the display off after BacklightOffAfterMS

feat(rpc): add methods to get and set BacklightSettings

bec1443

WIP: feat(settings): add Max backlight setting

f4d88c7

Nevexo force-pushed the nevexo/display-brightness branch from 0ef5b7e to f4d88c7 Compare January 3, 2025 22:15

Nevexo added 3 commits January 4, 2025 22:22

chore: use constant for backlight control file

db4c0c7

fix: only attempt to wake the display if it's off

a267bb3

feat(display): wake on touch

74cdeca

Nevexo added 2 commits January 4, 2025 22:26

fix: re-use buffer between reads

d6e4df2

fix: wakeDisplay() on start to fix warm start issue

7e7310b

If the application had turned off the display before exiting, it wouldn't be brought on when the application restarted without a device reboot.

tutman96 reviewed Jan 5, 2025

View reviewed changes

display.go Show resolved Hide resolved

tutman96 reviewed Jan 5, 2025

View reviewed changes

Nevexo commented Jan 5, 2025

View reviewed changes

display.go Outdated Show resolved Hide resolved

chore: various comment & string updates

1fe71da

eric reviewed Jan 5, 2025

View reviewed changes

display.go Outdated Show resolved Hide resolved

fix: newline on set brightness log

e9b5390

Noticed by @eric jetkvm#17 (comment)

Nevexo mentioned this pull request Jan 7, 2025

Option to flip the display upside down and reduce brightness (or night mode) #43

Open

Nevexo mentioned this pull request Jan 14, 2025

[Feature Request] Auto turn off display on JetKVM device after a period of inactivity #75

Closed

beornlake mentioned this pull request Jan 18, 2025

[feature request] Configurable display brightness #82

Closed

Nevexo added 2 commits January 20, 2025 18:38

fix: set default value for display

daaddef

Set the DisplayMaxBrightness to the default brightness used out-of-the-box by JetKVM. Also sets the dim/timeout to 2 minutes and 30 mintes respectively.

feat(display.go): use tickers to countdown to dim/off

79bac39

As suggested by tutman in jetkvm#17, use tickers set to the duration of dim/off to avoid a loop running every second. The tickers are reset to the dim/off times whenever wakeDisplay() is called.

Nevexo added 2 commits January 27, 2025 20:51

chore: update config

34e42fd

Changed Dim & Off values to seconds instead of milliseconds, there's no need for it to be that precise.

feat(display.go): wakeDisplay() force

e9f140c

Adds the force boolean to wakedisplay() which allows skipping the backlightState == 0 check, this means you can force a ticker reset, even if the display is currently in the "full bright" state

joshuasing reviewed Jan 28, 2025

View reviewed changes

Nevexo added 2 commits January 28, 2025 09:10

chore: update variable names

a05df7a

As part of @joshuasing's review on jetkvm#17, updated variables & constants to match the Go best practices. Signed-off-by: Cameron Fleming <[email protected]>

fix(display): move backlightTicker setup into screen setup goroutine

6445628

Signed-off-by: Cameron Fleming <[email protected]>

Nevexo mentioned this pull request Jan 28, 2025

Feature request: ability to put device screen to sleep #110

Closed

chore: fix some start-up timing issues

f5035f2

Nevexo added a commit to Nevexo/jetkvm-kvm that referenced this pull request Jan 28, 2025

chore: update variable names

c3a0604

As part of @joshuasing's review on jetkvm#17, updated variables & constants to match the Go best practices. Signed-off-by: Cameron Fleming <[email protected]>

Nevexo added 3 commits January 28, 2025 21:49

fix(display): Don't attempt to start the tickers if the display is di…

9896eba

…sabled If max_brightness is zero, then there's no point in trying to dim it (or turn it off...)

fix: wakeDisplay() doesn't need to stop the tickers

8071f81

The tickers only need to be reset, if they're disabled, they won't have been started.

fix: Don't wake up the display if it's turned off

ce54d10

joshuasing reviewed Jan 30, 2025

View reviewed changes

Nevexo mentioned this pull request Feb 10, 2025

[FEATURE REQUEST] Add hability to either dim or turn off JetKVM screen after timeout or schedule time #145

Closed

Nevexo added a commit to Nevexo/jetkvm-kvm that referenced this pull request Feb 11, 2025

chore: update variable names

d19f688

As part of @joshuasing's review on jetkvm#17, updated variables & constants to match the Go best practices. Signed-off-by: Cameron Fleming <[email protected]>

Nevexo added a commit to Nevexo/jetkvm-kvm that referenced this pull request Feb 11, 2025

chore: update variable names

86725ce

As part of @joshuasing's review on jetkvm#17, updated variables & constants to match the Go best practices. Signed-off-by: Cameron Fleming <[email protected]>

Merge branch 'dev' into nevexo/display-brightness

e177fdb

chemhack changed the title ~~WIP: feat(display.go): Add display brightness settings~~ feat(display.go): Add display brightness settings Feb 17, 2025

chemhack merged commit 5217377 into jetkvm:dev Feb 17, 2025
1 check passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(display.go): Add display brightness settings #17

feat(display.go): Add display brightness settings #17

Nevexo commented Jan 3, 2025 •

edited

Loading

Nevexo commented Jan 3, 2025

Nevexo commented Jan 4, 2025

Nevexo commented Jan 4, 2025

tutman96 Jan 5, 2025

Nevexo Jan 5, 2025

Nevexo Jan 20, 2025

Nevexo commented Jan 27, 2025

lwbt commented Jan 27, 2025

Nevexo commented Jan 27, 2025 •

edited

Loading

lwbt commented Jan 27, 2025

Nevexo commented Jan 27, 2025

Nevexo commented Jan 27, 2025

joshuasing left a comment

joshuasing Jan 28, 2025

Nevexo Jan 28, 2025

joshuasing Jan 28, 2025

Nevexo Jan 28, 2025

joshuasing Jan 30, 2025 •

edited

Loading

Nevexo Jan 30, 2025

CLAassistant commented Feb 13, 2025 •

edited

Loading

Mauker1 commented Feb 17, 2025

Nevexo commented Feb 17, 2025

chemhack commented Feb 17, 2025

Nevexo commented Feb 17, 2025

Mauker1 commented Feb 17, 2025

		TOUCHSCREEN_DEVICE string = "/dev/input/event1"
		BACKLIGHT_CONTROL_CLASS string = "/sys/class/backlight/backlight/brightness"

feat(display.go): Add display brightness settings #17

feat(display.go): Add display brightness settings #17

Conversation

Nevexo commented Jan 3, 2025 • edited Loading

Settings UI for these changes

Nevexo commented Jan 3, 2025

Nevexo commented Jan 4, 2025

Nevexo commented Jan 4, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Nevexo commented Jan 27, 2025

lwbt commented Jan 27, 2025

Nevexo commented Jan 27, 2025 • edited Loading

lwbt commented Jan 27, 2025

Nevexo commented Jan 27, 2025

Nevexo commented Jan 27, 2025

joshuasing left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

joshuasing Jan 30, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

CLAassistant commented Feb 13, 2025 • edited Loading

Mauker1 commented Feb 17, 2025

Nevexo commented Feb 17, 2025

chemhack commented Feb 17, 2025

Nevexo commented Feb 17, 2025

Mauker1 commented Feb 17, 2025

Nevexo commented Jan 3, 2025 •

edited

Loading

Nevexo commented Jan 27, 2025 •

edited

Loading

joshuasing Jan 30, 2025 •

edited

Loading

CLAassistant commented Feb 13, 2025 •

edited

Loading