Conversation
ben16w
changed the title
There was a problem hiding this comment.
Pull Request Overview
This PR introduces a brightness control utility to restore brightness on device resume. The implementation provides platform-specific brightness management for three different handheld gaming devices.
Key changes:
- Added a new Go program (
set-brightness.go) that sets brightness levels for tg5040, miyoomini, and rg35xxplus platforms - Updated build system to compile platform-specific binaries with embedded platform identifiers
Reviewed Changes
Copilot reviewed 2 out of 5 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
| src/set-brightness.go | New brightness control utility with platform-specific implementations using ioctl and sysfs interfaces |
| Makefile | Build targets for platform-specific set-brightness binaries with compile-time platform identification |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| } | ||
|
|
||
| func applybrightnessDevice(val int) error { | ||
| const brightnessDevice = "/sys/class/pwm/pwmchip0/pwm0/brightnessDevice" |
There was a problem hiding this comment.
The sysfs path appears incorrect. The standard PWM sysfs interface uses 'duty_cycle' not 'brightnessDevice'. This path likely doesn't exist and will cause the function to fail.
| const brightnessDevice = "/sys/class/pwm/pwmchip0/pwm0/brightnessDevice" | |
| const brightnessDevice = "/sys/class/pwm/pwmchip0/pwm0/duty_cycle" |
| fmt.Errorf("Failed to open %s: %w", brightnessDevice, err) | ||
| return err | ||
| } | ||
| defer file.Close() | ||
|
|
||
| _, err = fmt.Fprintf(file, "%d", val) | ||
| if err != nil { | ||
| fmt.Errorf("Failed to set brightness: %w", err) | ||
| return err |
There was a problem hiding this comment.
fmt.Errorf return value is not being used. Should be 'return fmt.Errorf(...)' or use fmt.Printf/fmt.Println to output the error.
| fmt.Errorf("Failed to open %s: %w", brightnessDevice, err) | |
| return err | |
| } | |
| defer file.Close() | |
| _, err = fmt.Fprintf(file, "%d", val) | |
| if err != nil { | |
| fmt.Errorf("Failed to set brightness: %w", err) | |
| return err | |
| return fmt.Errorf("Failed to open %s: %w", brightnessDevice, err) | |
| } | |
| defer file.Close() | |
| _, err = fmt.Fprintf(file, "%d", val) | |
| if err != nil { | |
| return fmt.Errorf("Failed to set brightness: %w", err) |
| fmt.Errorf("Failed to set brightness: %w", err) | ||
| return err | ||
| } |
There was a problem hiding this comment.
fmt.Errorf return value is not being used. Should be 'return fmt.Errorf(...)' or use fmt.Printf/fmt.Println to output the error.
| fmt.Errorf("Failed to set brightness: %w", err) | |
| return err | |
| } | |
| return fmt.Errorf("Failed to set brightness: %w", err) | |
| } |
| mapTrimui := [11]int{0, 1, 8, 16, 32, 48, 72, 96, 128, 176, 255} | ||
| raw = mapTrimui[value] |
There was a problem hiding this comment.
Missing bounds check before array access. If 'value' is outside 0-10 range, this will panic with index out of range. Add validation or use a bounds check.
| mapRg35xxplus := [11]int{0, 4, 6, 10, 16, 32, 48, 64, 96, 160, 255} | ||
| raw = mapRg35xxplus[value] |
There was a problem hiding this comment.
Missing bounds check before array access. If 'value' is outside 0-10 range, this will panic with index out of range. Add validation or use a bounds check.
closes #4