feat: HTTP API to control surface brightness and read surface info - #4293
feat: HTTP API to control surface brightness and read surface info#4293FusselTV wants to merge 2 commits into
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughAdds two new HTTP API endpoints for surfaces: ChangesSurfaces HTTP API endpoints
🚥 Pre-merge checks | ✅ 3 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (3 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
companion/test/Service/HttpApi.test.ts (1)
203-232: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winPlease add a
?brightness=regression case.The handler has a dedicated
rawValue === ''branch, but this block only covers missing, non-numeric, and out-of-range inputs. A tiny test for an empty query string would lock down that validation path too.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: fb916af1-f027-4cb6-b523-2abe43455dc4
📒 Files selected for processing (5)
companion/lib/Service/HttpApi.tscompanion/lib/Service/ServiceApi.tscompanion/lib/Surface/Controller.tscompanion/test/Service/HttpApi.test.tsdocs/user-guide/5_remote-control/http-remote-control.md
|
Yeah I think this should be held back and done on top of #4259 once that is merged, instead of building out the soon to be 'old' (or maybe the 'simplified' api) |
Adds HTTP endpoints to list surfaces (with their state: name, connection, size, brightness and current page) and to set a surface's brightness, as a suggested way to close #1964. Documented in a new section of
docs/user-guide/5_remote-control/http-remote-control.md.Use cases:
It extends the existing remote-control HTTP API and follows its patterns: the surface list projects a minimal shape like
/api/connectionsandsetDeviceBrightnessnow returns a boolean (likepressControlandrestartConnection) so the endpoint can return a404when no connected surface matches the id. The tests were written with Claude Opus.Note that #4259 proposes a new, more formal REST API and currently covers connections only. If that becomes the preferred direction and is later extended to surfaces, these endpoints could be reimplemented there and this PR dropped. Happy to go either way.
Related to #1964, #4266 and #4273.
Summary by CodeRabbit
New Features
Bug Fixes
Documentation