Conversation
- Add `devices rename <name-or-id> <new-name>` command - Add `zones rename <name-or-id> <new-name>` command - Add UpdateDevice and UpdateZone client methods - Add unit tests for rename commands and client methods
- Add `zones icons` command to list available icons - Add `zones set-icon <zone> <icon>` command to change zone icon - Add `--icon` flag to `zones rename` command for combined rename+icon change - Include list of 34 known Homey zone icons - Add tests for new commands
Split large command files for better maintainability: devices.go (185 lines) - types, list, get, values, findDevice helper devices_control.go (113 lines) - set, on, off devices_manage.go (62 lines) - rename, delete devices_settings.go (108 lines) - get-settings, set-setting zones.go (144 lines) - types, list, icons, findZone helper zones_manage.go (110 lines) - rename, set-icon, delete Added shared helper functions (findDevice, findZone, parseValue) to reduce code duplication across commands.
Devices: - move: relocate device to a different zone - set-note: add/update device notes - set-icon: customize device icon - hide/unhide: control device visibility in UI Device Groups: - list: show all device groups - create: create new group with devices - update: modify group name or members - remove-device: remove device from group Zones: - get: retrieve single zone details - create: create new zone under parent - move: relocate zone to different parent All commands support both table and JSON output formats.
New commands: - Flow folders: list, get, create, update, delete - Apps: install, uninstall, enable, disable, update, settings, usage - Users: list, get, me, create, delete - Moods: list, get, create, update, delete, set - Dashboards: list, get, create, update, delete - Presence: get/set present, get/set asleep for users - Weather: current conditions and hourly forecast - Energy: yearly report, currency, delete reports - System: get/set Homey name - Insights: delete logs, clear entries - Notifications: delete, clear all, list owners Added tests for all new API client methods and commands.
README.md: - Restructured with clear sections and better organization - Added all new commands: presence, moods, dashboards, weather, flow folders, extended apps/users/energy/insights/system - Added device management: rename, move, hide, icons, notes - Added zone management: create, rename, move, icons - Improved quick start and configuration sections AI documentation (homeyctl ai): - Organized by access level: read-only, control, write/delete - Clear token security guidance for AI assistants - Complete command reference for all features - Updated flow JSON format and examples
📝 WalkthroughWalkthroughThis PR dramatically expands the Homey CLI tool by adding comprehensive command support for managing smart home resources including devices, apps, dashboards, flows, moods, users, weather, presence, notifications, energy, and system settings. It includes new API client methods and extensive test coverage, along with updated documentation emphasizing AI-safe token handling. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 13
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
cmd/devices.go (1)
119-124: Use map keys instead of cap.ID for capability identifiers.
CapabilitiesObjismap[string]Capabilitykeyed by capability ID. If the API response doesn't include anidfield in each capability object,cap.IDwill be empty, resulting in blank table entries and malformed JSON output. Use the map key directly.🔧 Proposed fix
- for _, cap := range device.CapabilitiesObj { - fmt.Fprintf(w, " %s\t%v\n", cap.ID, cap.Value) - } + for id, cap := range device.CapabilitiesObj { + fmt.Fprintf(w, " %s\t%v\n", id, cap.Value) + } ... - for _, cap := range device.CapabilitiesObj { - fmt.Fprintf(w, "%s\t%v\n", cap.ID, cap.Value) - } + for id, cap := range device.CapabilitiesObj { + fmt.Fprintf(w, "%s\t%v\n", id, cap.Value) + } ... - for _, cap := range device.CapabilitiesObj { - values[cap.ID] = cap.Value - } + for id, cap := range device.CapabilitiesObj { + values[id] = cap.Value + }
🤖 Fix all issues with AI agents
In `@cmd/apps_manage.go`:
- Around line 31-40: The json.Unmarshal call in the branch that checks
isTableFormat() ignores its returned error which can lead to empty
app.Name/app.Version and confusing output; update the code around the
json.Unmarshal(result, &app) call to capture the error, and if non-nil return or
log a user-friendly error (including the unmarshalling error and possibly the
raw result) instead of proceeding to fmt.Printf("Installed app: %s v%s\n",
app.Name, app.Version); reference the isTableFormat() branch, the local variable
app, the json.Unmarshal call, and the fmt.Printf so you validate the unmarshal
before printing and return an appropriate error or message.
- Around line 104-132: The apps update command currently always reads the
"autoupdate" boolean flag and sends it in the updates payload, causing
unintended changes when the flag wasn't provided; modify the RunE for
appsUpdateCmd to call cmd.Flags().Changed("autoupdate") and only include the
"autoupdate" key in the updates map if Changed(...) is true (or return a
user-facing error when no flags were provided), so apiClient.UpdateApp receives
only explicitly set fields; update references in RunE where
cmd.Flags().GetBool("autoupdate"), the updates map, and the apiClient.UpdateApp
call accordingly.
In `@cmd/dashboards.go`:
- Around line 47-80: The JSON output currently returns the raw map from
apiClient.GetDashboards() instead of a flat array; update the RunE of
dashboardsListCmd to unmarshal the API response into a map[string]Dashboard (as
already done for table output), convert that map's values into a []Dashboard
slice (preserve iteration order if needed or sort by key/name), then marshal
that slice and pass the marshaled bytes to outputJSON (or call outputJSON with
the slice if it accepts interface{}), ensuring the JSON path uses the same
Dashboard type and error handling as the existing json.Unmarshal and fmt.Errorf
patterns.
In `@cmd/devices_control.go`:
- Around line 9-25: The parseValue function should use strconv.ParseBool for
boolean detection and strconv.ParseFloat (with 64-bit) to validate numeric input
instead of fmt.Sscanf; update parseValue to first attempt
strconv.ParseBool(valueStr) and return the bool if no error, then attempt
strconv.ParseFloat(valueStr, 64) and return the float64 if no error, otherwise
fall back to returning the original string — this ensures the entire string is
validated and rejects partial/malformed numeric inputs.
In `@cmd/flows_folders.go`:
- Around line 47-83: flowsFoldersListCmd currently returns the raw map payload
for JSON output which serializes as an object keyed by IDs; unmarshal the
response from apiClient.GetFlowFolders() into map[string]FlowFolder (same as in
the table branch), convert that map into a flat []FlowFolder slice (preserving
order if needed), then marshal or pass that slice to outputJSON so the command
emits a flat JSON array; update the JSON-path code path in flowsFoldersListCmd
(and reuse the existing json.Unmarshal logic) instead of calling
outputJSON(data) with the original bytes.
In `@cmd/moods.go`:
- Around line 144-148: Replace the platform-specific os.ReadFile("/dev/stdin")
call with a cross-platform read from os.Stdin using io.ReadAll; specifically, in
the branch that checks args[1] == "-" read into the existing variables (data,
err) via io.ReadAll(os.Stdin) instead of os.ReadFile, ensure the io package is
imported, and leave the other branch using os.ReadFile(args[1]) unchanged so
data and err are handled uniformly.
In `@cmd/notifications.go`:
- Around line 52-79: The JSON output path currently returns the raw map bytes (a
map keyed by ID) instead of a flat array; unmarshal data into the same
map[string]Notification you use for the table branch (notifications), convert
its values into a []Notification slice (preserving whatever order you need),
then marshal that slice and pass the resulting flat-array bytes to outputJSON
(or pass the slice into outputJSON if it accepts an interface). Update the
non-table branch after the isTableFormat() block to perform this conversion so
notify list emits a flat JSON array.
In `@cmd/presence.go`:
- Around line 161-203: The json.Unmarshal calls for the GetUserMe and GetAsleep
responses are missing error handling; update the block that handles nameOrID ==
"me" to capture and check the error returned by json.Unmarshal(data, &u) (where
u is the temporary struct used for ID/Name) and return a descriptive error if
unmarshalling fails, and similarly check the error returned by
json.Unmarshal(data, &asleep) (where asleep is the temporary struct used for
Value) before using asleep.Value; ensure both error returns surface useful
context (e.g., which API response failed) and keep existing control flow that
prints table format or calls outputJSON(data).
- Around line 31-92: The JSON unmarshal calls for the user, present, and asleep
responses currently ignore errors; update the presence command to check and
return errors from json.Unmarshal for the anonymous user struct (after
apiClient.GetUserMe()), and for the present and asleep structs (after
apiClient.GetPresent(userID) and apiClient.GetAsleep(userID)), so parsing
failures propagate and the command fails fast; locate the json.Unmarshal
invocations that populate the anonymous user struct (ID/Name), the present
variable, and the asleep variable and add error handling that returns the
unmarshal error (or a wrapped error with context) instead of proceeding with
potentially empty values.
In `@cmd/users_manage.go`:
- Around line 233-256: The loop over users currently ignores errors from
apiClient.GetPresent/GetAsleep and json.Unmarshal which can hide API failures;
update the code in the users iteration (the block using apiClient.GetPresent,
apiClient.GetAsleep, the present/asleep structs and json.Unmarshal) to check and
handle both API and unmarshal errors: if GetPresent/GetAsleep returns an error
or unmarshalling fails, log a warning (or write to stderr) and set the
corresponding presentStr/asleepStr to "unknown" instead of defaulting to "no",
otherwise use the unmarshalled Value to choose "yes"/"no"; keep the final
fmt.Fprintf(w, ...) and w.Flush() but ensure error paths are handled before
formatting.
- Around line 125-135: The update path currently accepts any role string while
the create path restricts roles via the validRoles check (variables
validRoles/isValid and the role variable); add the same validation logic to the
update command handler (reuse validRoles and the isValid loop) and return the
same formatted error if invalid, and then make the flag help text consistent
(remove or add "owner" so both create and update advertise the same supported
roles). Ensure you update the update command's role parsing code (the block
around where role is read at the update handler) and the flag help string that
currently mentions "owner" so both commands match.
In `@internal/client/client.go`:
- Around line 611-644: GetEnergyReportYear currently always appends
"?year="+year which sends an empty query when year is empty; change
GetEnergyReportYear to build the request path conditionally: if year is
non-empty, append "?year="+url.QueryEscape(year) to
"/api/manager/energy/report/year", otherwise call doRequest with just
"/api/manager/energy/report/year". Reference the GetEnergyReportYear method and
use url.QueryEscape to properly escape the year parameter.
In `@README.md`:
- Around line 423-426: Update the fenced code block that shows the CORRECT/WRONG
droptoken examples to include a language tag (use "text") so markdownlint stops
flagging it; locate the snippet containing the strings "CORRECT:
\"homey:device:abc123|measure_temperature\"" and "WRONG:
\"homey:device:abc123:measure_temperature\"" and change the opening fence from
``` to ```text while keeping the closing fence as ``` to improve linting and
readability.
🧹 Nitpick comments (16)
cmd/zones.go (1)
161-183: Use outputJSON for JSON mode and handle marshal errors.Right now JSON output bypasses the shared helper and ignores marshal errors. Align with the standard JSON path for consistency.
As per coding guidelines, JSON output should go through outputJSON().♻️ Suggested fix
- jsonData, _ := json.MarshalIndent(KnownZoneIcons, "", " ") - fmt.Println(string(jsonData)) - return nil + jsonData, err := json.Marshal(KnownZoneIcons) + if err != nil { + return fmt.Errorf("failed to marshal zone icons: %w", err) + } + outputJSON(jsonData) + return nilcmd/devices_settings.go (1)
54-101: Add JSON output path forset-settingto honor format flag.
set-settingalways prints a human string even when JSON output is requested, which diverges from the common output-format contract. Consider emitting structured JSON when not in table mode.As per coding guidelines, please ensure command outputs support JSON/table formatting.
♻️ Suggested adjustment
@@ - fmt.Printf("Set %s setting %s = %v\n", device.Name, settingKey, value) - return nil + if isTableFormat() { + fmt.Printf("Set %s setting %s = %v\n", device.Name, settingKey, value) + return nil + } + outputJSON(map[string]interface{}{ + "device": device.Name, + "setting": settingKey, + "value": value, + }) + return nil }, }cmd/insights.go (1)
134-222: Consider extracting the repeated log lookup logic into a helper function.Both
insightsDeleteCmdandinsightsClearCmdcontain nearly identical code for looking up a log by ID (fetching all logs, parsing, iterating to find match). This pattern is already abstracted inusers_manage.gowithfindUser().♻️ Proposed refactor to extract helper
+// findInsightLog finds an insight log by ID and returns its metadata +func findInsightLog(logID string) (ownerURI, ownerID, title string, err error) { + data, err := apiClient.GetInsights() + if err != nil { + return "", "", "", err + } + + var logs []InsightLog + if err := json.Unmarshal(data, &logs); err != nil { + return "", "", "", fmt.Errorf("failed to parse insights: %w", err) + } + + for _, log := range logs { + if log.ID == logID { + return log.OwnerURI, log.OwnerID, log.Title, nil + } + } + + return "", "", "", fmt.Errorf("log not found: %s", logID) +}Then simplify the commands:
var insightsDeleteCmd = &cobra.Command{ // ... RunE: func(cmd *cobra.Command, args []string) error { - logID := args[0] - - // Look up the log to get ownerUri and ownerId - data, err := apiClient.GetInsights() - if err != nil { - return err - } - - var logs []InsightLog - if err := json.Unmarshal(data, &logs); err != nil { - return fmt.Errorf("failed to parse insights: %w", err) - } - - var ownerURI, ownerID, title string - for _, log := range logs { - if log.ID == logID { - ownerURI = log.OwnerURI - ownerID = log.OwnerID - title = log.Title - break - } - } - - if ownerURI == "" { - return fmt.Errorf("log not found: %s", logID) - } + ownerURI, ownerID, title, err := findInsightLog(args[0]) + if err != nil { + return err + } if err := apiClient.DeleteInsightLog(ownerURI, ownerID); err != nil { return err } // ... }, }internal/client/client_test.go (1)
564-595: Consider verifying the request body inTestSetSystemName.Unlike other setter tests (
TestSetPresent,TestSetAsleep), this test doesn't verify that the new system name is actually sent in the request body. This could miss bugs where the name isn't properly serialized.♻️ Suggested improvement
func TestSetSystemName(t *testing.T) { var receivedPath string var receivedMethod string + var receivedBody []byte server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { receivedPath = r.URL.Path receivedMethod = r.Method + receivedBody, _ = io.ReadAll(r.Body) w.WriteHeader(http.StatusOK) w.Write([]byte(`{}`)) })) defer server.Close() // ... client setup ... err := client.SetSystemName("New Homey Name") if err != nil { t.Fatalf("SetSystemName failed: %v", err) } // ... existing assertions ... + // Verify the name was sent + if !strings.Contains(string(receivedBody), "New Homey Name") { + t.Errorf("expected body to contain 'New Homey Name', got %s", string(receivedBody)) + } }cmd/zones_manage.go (2)
171-188: Consider adding a confirmation prompt for destructive operations.The delete command immediately deletes the zone without confirmation. For a potentially destructive operation that may affect nested zones and devices, consider adding a
--forceflag or interactive confirmation prompt.
138-169: Add validation to prevent circular zone references in move operation.The move command does not check if the new parent is the zone itself or one of its descendants. This could create circular references in the zone hierarchy. Add client-side validation before calling
apiClient.UpdateZone():// Check if attempting to move a zone under itself or its descendants if zone.ID == newParent.ID { return fmt.Errorf("cannot move a zone under itself") } // Check for circular reference by traversing ancestors of newParent // to ensure zone is not an ancestor of newParentWhile the Homey API may reject such requests server-side, a client-side check provides immediate feedback and prevents confusing API errors.
cmd/zones_test.go (1)
1-142: Consider adding tests for thecreateandmovecommands.The test file covers
rename,delete,list,icons, andset-icon, but thecreateandmovecommands introduced inzones_manage.godon't have corresponding existence tests here.Would you like me to generate the additional test cases for
createandmovecommands?cmd/devices_groups.go (2)
184-189: Consider validating that the found device is actually a group.The update command uses
findDevicewhich returns any device, but the operation is only meaningful for device groups (devices withvirtualClass == "group"). Operating on a non-group device could produce confusing behavior or API errors.♻️ Suggested validation
RunE: func(cmd *cobra.Command, args []string) error { // Find the group (it's a device with virtualClass "group") device, err := findDevice(args[0]) if err != nil { return err } + + // Verify it's actually a group + data, err := apiClient.GetDevices() + if err != nil { + return err + } + var devices map[string]json.RawMessage + if err := json.Unmarshal(data, &devices); err != nil { + return fmt.Errorf("failed to parse devices: %w", err) + } + if raw, ok := devices[device.ID]; ok { + var d struct { VirtualClass string `json:"virtualClass"` } + if err := json.Unmarshal(raw, &d); err == nil && d.VirtualClass != "group" { + return fmt.Errorf("'%s' is not a device group", device.Name) + } + }Alternatively, consider creating a
findDeviceGrouphelper that combinesfindDevicewith group validation.
284-310: Same concern:remove-deviceshould validate that the target is a group.The
findDevicecall on line 293 doesn't verify that the found device is actually a device group. Consider adding validation or extracting a sharedfindDeviceGrouphelper.cmd/dashboards.go (1)
193-210: Consider adding a confirmation prompt for delete.Like the zone delete command, this immediately deletes without confirmation. For a destructive operation on a potentially complex dashboard, a
--forceflag or confirmation could prevent accidental data loss.cmd/apps.go (1)
27-46: ReusefindAppto remove duplicated lookup logic.The helper is great, but
appsGetCmdandappsRestartCmdstill duplicate the same lookup/parsing. Consider reusingfindAppto keep matching behavior consistent and reduce maintenance surface.[methods to update outside this range]
♻️ Suggested refactor
@@ - data, err := apiClient.GetApps() - if err != nil { - return err - } - - var apps map[string]App - if err := json.Unmarshal(data, &apps); err != nil { - return fmt.Errorf("failed to parse apps: %w", err) - } - - // Find app by name or ID - var appID string - for _, a := range apps { - if a.ID == nameOrID || strings.EqualFold(a.Name, nameOrID) { - appID = a.ID - break - } - } - - if appID == "" { - return fmt.Errorf("app not found: %s", nameOrID) - } - - appData, err := apiClient.GetApp(appID) + app, err := findApp(nameOrID) + if err != nil { + return err + } + + appData, err := apiClient.GetApp(app.ID) @@ - data, err := apiClient.GetApps() - if err != nil { - return err - } - - var apps map[string]App - if err := json.Unmarshal(data, &apps); err != nil { - return fmt.Errorf("failed to parse apps: %w", err) - } - - // Find app by name or ID - var app *App - for _, a := range apps { - if a.ID == nameOrID || strings.EqualFold(a.Name, nameOrID) { - app = &a - break - } - } - - if app == nil { - return fmt.Errorf("app not found: %s", nameOrID) - } + app, err := findApp(nameOrID) + if err != nil { + return err + }cmd/moods.go (2)
70-84: Consider sorting moods for deterministic table output.Map iteration order in Go is non-deterministic. For consistent CLI output, consider collecting moods into a slice and sorting by name before printing.
Proposed fix
+import "sort" + if len(moods) == 0 { fmt.Println("No moods found.") return nil } +// Convert to slice and sort for consistent output +moodList := make([]Mood, 0, len(moods)) +for _, m := range moods { + moodList = append(moodList, m) +} +sort.Slice(moodList, func(i, j int) bool { + return moodList[i].Name < moodList[j].Name +}) + w := tabwriter.NewWriter(os.Stdout, 0, 0, 2, ' ', 0) fmt.Fprintln(w, "NAME\tPRESET\tACTIVE\tID") fmt.Fprintln(w, "----\t------\t------\t--") -for _, m := range moods { +for _, m := range moodList {
93-111:moods getcommand always outputs JSON regardless of format setting.Unlike other commands that branch on
isTableFormat(), this command always callsoutputJSON(). Consider adding table format support for consistency, or document this as intentional behavior for detailed mood data.cmd/apps_manage.go (1)
139-157:apps settings listcommand doesn't support table format.Unlike other list-style commands, this always outputs JSON. Consider adding table format support for consistency with the rest of the CLI, or document this as intentional.
cmd/devices.go (2)
37-55: Disambiguate name matches and prefer direct ID lookup.
If Homey allows duplicate device names,findDevicecan return a random device because map iteration order is undefined. Consider first checkingdevices[nameOrID](exact ID lookup), then collecting name matches and erroring on ambiguity.♻️ Suggested approach
- for _, d := range devices { - if d.ID == nameOrID || strings.EqualFold(d.Name, nameOrID) { - return &d, nil - } - } - return nil, fmt.Errorf("device not found: %s", nameOrID) + if d, ok := devices[nameOrID]; ok { + return &d, nil + } + var matches []Device + for _, d := range devices { + if strings.EqualFold(d.Name, nameOrID) { + matches = append(matches, d) + } + } + if len(matches) == 1 { + return &matches[0], nil + } + if len(matches) > 1 { + ids := make([]string, 0, len(matches)) + for _, m := range matches { + ids = append(ids, m.ID) + } + return nil, fmt.Errorf("multiple devices named %q: %s", nameOrID, strings.Join(ids, ", ")) + } + return nil, fmt.Errorf("device not found: %s", nameOrID)If Homey enforces unique device names, documenting that assumption would also be fine.
86-99: Route JSON output throughoutputJSON()for consistency.
The JSON branches manually marshal and ignore errors. The guidelines call foroutputJSON()usage; it centralizes formatting and error handling. As per coding guidelines, please update these outputs to useoutputJSON(...).Example adjustment
- out, _ := json.MarshalIndent(filtered, "", " ") - fmt.Println(string(out)) - return nil + return outputJSON(filtered)Also applies to: 129-174
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (37)
.gitignoreREADME.mdcmd/ai.gocmd/apps.gocmd/apps_manage.gocmd/apps_test.gocmd/dashboards.gocmd/dashboards_test.gocmd/devices.gocmd/devices_control.gocmd/devices_groups.gocmd/devices_manage.gocmd/devices_settings.gocmd/devices_test.gocmd/energy.gocmd/energy_test.gocmd/flows_folders.gocmd/flows_folders_test.gocmd/insights.gocmd/insights_test.gocmd/moods.gocmd/moods_test.gocmd/notifications.gocmd/notifications_test.gocmd/presence.gocmd/presence_test.gocmd/system.gocmd/system_test.gocmd/users_manage.gocmd/users_test.gocmd/weather.gocmd/weather_test.gocmd/zones.gocmd/zones_manage.gocmd/zones_test.gointernal/client/client.gointernal/client/client_test.go
🧰 Additional context used
📓 Path-based instructions (4)
cmd/**/*.go
📄 CodeRabbit inference engine (CLAUDE.md)
cmd/**/*.go: Each command group should be defined in a separate file in thecmd/directory (e.g.,cmd/newresource.go) following the pattern:homeyctl <resource> <action> [args]
Support both JSON and table output viaisTableFormat()andoutputJSON()helper functions in command implementations
All list commands must return flat JSON arrays for easy parsing
Files:
cmd/devices_settings.gocmd/zones_test.gocmd/energy_test.gocmd/apps.gocmd/presence.gocmd/moods_test.gocmd/presence_test.gocmd/insights_test.gocmd/system.gocmd/flows_folders.gocmd/weather.gocmd/energy.gocmd/dashboards_test.gocmd/devices_test.gocmd/system_test.gocmd/zones.gocmd/devices_groups.gocmd/notifications_test.gocmd/users_manage.gocmd/zones_manage.gocmd/ai.gocmd/devices_control.gocmd/weather_test.gocmd/moods.gocmd/apps_manage.gocmd/apps_test.gocmd/dashboards.gocmd/insights.gocmd/flows_folders_test.gocmd/users_test.gocmd/notifications.gocmd/devices.gocmd/devices_manage.go
**/*.go
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.go: Use pipe (|) format for droptoken, not colon:homey:device:<id>|measure_temperature
PUT requests perform partial/merge updates - explicitly set empty array to remove conditions/actions:"conditions": []
**/*.go: Format code with gofumpt (runmake fmt)
Run golangci-lint for linting (runmake lint)
Follow standard Go naming conventions: exported names use CamelCase, unexported names use camelCase
Files:
cmd/devices_settings.gocmd/zones_test.gocmd/energy_test.gocmd/apps.gocmd/presence.gocmd/moods_test.gocmd/presence_test.gocmd/insights_test.gocmd/system.gocmd/flows_folders.gocmd/weather.gocmd/energy.gocmd/dashboards_test.gocmd/devices_test.gocmd/system_test.gocmd/zones.gointernal/client/client_test.gocmd/devices_groups.gocmd/notifications_test.gocmd/users_manage.gocmd/zones_manage.gocmd/ai.gocmd/devices_control.gocmd/weather_test.gocmd/moods.gocmd/apps_manage.gocmd/apps_test.gocmd/dashboards.gocmd/insights.gocmd/flows_folders_test.gocmd/users_test.gocmd/notifications.gocmd/devices.gocmd/devices_manage.gointernal/client/client.go
**/*_test.go
📄 CodeRabbit inference engine (AGENTS.md)
**/*_test.go: Use Go'stestingpackage for tests
Rungo test ./...before shipping changes
Tests should live alongside code as*_test.gofiles
Files:
cmd/zones_test.gocmd/energy_test.gocmd/moods_test.gocmd/presence_test.gocmd/insights_test.gocmd/dashboards_test.gocmd/devices_test.gocmd/system_test.gointernal/client/client_test.gocmd/notifications_test.gocmd/weather_test.gocmd/apps_test.gocmd/flows_folders_test.gocmd/users_test.go
internal/client/**/*.go
📄 CodeRabbit inference engine (CLAUDE.md)
internal/client/**/*.go: All API calls must go throughClient.doRequest()which handles auth headers and error responses
API client should returnjson.RawMessagefor GET requests to allow commands to parse only what they need
Files:
internal/client/client_test.gointernal/client/client.go
🧠 Learnings (12)
📓 Common learnings
Learnt from: CR
Repo: langtind/homeyctl PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-15T22:36:58.411Z
Learning: This is a CLI tool for controlling Homey smart home devices via the local Homey API, built with Cobra for command handling and Viper for configuration
📚 Learning: 2026-01-15T22:36:58.411Z
Learnt from: CR
Repo: langtind/homeyctl PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-15T22:36:58.411Z
Learning: Applies to cmd/**/*.go : Each command group should be defined in a separate file in the `cmd/` directory (e.g., `cmd/newresource.go`) following the pattern: `homeyctl <resource> <action> [args]`
Applied to files:
cmd/devices_settings.go.gitignorecmd/presence.gocmd/moods_test.gocmd/presence_test.gocmd/insights_test.gocmd/system.gocmd/flows_folders.gocmd/weather.gocmd/system_test.gocmd/zones.gocmd/devices_groups.gocmd/users_manage.gocmd/zones_manage.gocmd/devices_control.gocmd/weather_test.gocmd/moods.gocmd/apps_manage.gocmd/apps_test.gocmd/dashboards.gocmd/flows_folders_test.gocmd/devices.gocmd/devices_manage.go
📚 Learning: 2026-01-15T22:36:58.411Z
Learnt from: CR
Repo: langtind/homeyctl PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-15T22:36:58.411Z
Learning: Applies to cmd/**/*.go : Support both JSON and table output via `isTableFormat()` and `outputJSON()` helper functions in command implementations
Applied to files:
cmd/devices_settings.gocmd/weather.gocmd/moods.gocmd/apps_manage.gocmd/dashboards.gocmd/notifications.go
📚 Learning: 2026-01-15T22:36:58.411Z
Learnt from: CR
Repo: langtind/homeyctl PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-15T22:36:58.411Z
Learning: This is a CLI tool for controlling Homey smart home devices via the local Homey API, built with Cobra for command handling and Viper for configuration
Applied to files:
cmd/devices_settings.go.gitignorecmd/system.gocmd/weather.goREADME.mdcmd/ai.gocmd/devices_control.gocmd/apps_manage.gocmd/devices.gocmd/devices_manage.go
📚 Learning: 2026-01-15T22:37:05.601Z
Learnt from: CR
Repo: langtind/homeyctl PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-15T22:37:05.601Z
Learning: Applies to **/*_test.go : Run `go test ./...` before shipping changes
Applied to files:
cmd/zones_test.gocmd/energy_test.gocmd/moods_test.gocmd/presence_test.gocmd/insights_test.gocmd/dashboards_test.gocmd/devices_test.gocmd/system_test.gointernal/client/client_test.gocmd/notifications_test.gocmd/weather_test.gocmd/apps_test.gocmd/flows_folders_test.gocmd/users_test.go
📚 Learning: 2026-01-15T22:37:05.601Z
Learnt from: CR
Repo: langtind/homeyctl PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-15T22:37:05.601Z
Learning: Applies to **/*_test.go : Use Go's `testing` package for tests
Applied to files:
cmd/zones_test.gocmd/moods_test.gocmd/presence_test.gocmd/insights_test.gocmd/devices_test.gocmd/system_test.gointernal/client/client_test.gocmd/weather_test.gocmd/apps_test.gocmd/flows_folders_test.gocmd/users_test.go
📚 Learning: 2026-01-15T22:36:58.411Z
Learnt from: CR
Repo: langtind/homeyctl PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-15T22:36:58.411Z
Learning: Applies to internal/config/**/*.go : Environment variables prefixed with `HOMEY_` should override config file values
Applied to files:
.gitignoreREADME.md
📚 Learning: 2026-01-15T22:37:05.601Z
Learnt from: CR
Repo: langtind/homeyctl PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-15T22:37:05.601Z
Learning: Configuration is managed via `~/.config/homeyctl/config.toml`, environment variables (`HOMEY_HOST`, `HOMEY_TOKEN`, `HOMEY_FORMAT`), or `--config` flag override
Applied to files:
.gitignoreREADME.mdcmd/ai.go
📚 Learning: 2026-01-15T22:36:58.411Z
Learnt from: CR
Repo: langtind/homeyctl PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-15T22:36:58.411Z
Learning: Applies to **/*.go : Use pipe (`|`) format for droptoken, not colon: `homey:device:<id>|measure_temperature`
Applied to files:
.gitignore
📚 Learning: 2026-01-15T22:36:58.411Z
Learnt from: CR
Repo: langtind/homeyctl PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-15T22:36:58.411Z
Learning: Applies to cmd/**/*.go : All list commands must return flat JSON arrays for easy parsing
Applied to files:
cmd/presence.gocmd/moods_test.gocmd/insights_test.gocmd/flows_folders.gocmd/weather.gocmd/dashboards_test.gocmd/devices_groups.gocmd/notifications_test.gocmd/users_manage.gocmd/moods.gocmd/apps_manage.gocmd/apps_test.gocmd/dashboards.gocmd/flows_folders_test.gocmd/users_test.gocmd/notifications.go
📚 Learning: 2026-01-15T22:37:05.601Z
Learnt from: CR
Repo: langtind/homeyctl PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-15T22:37:05.601Z
Learning: Applies to **/*_test.go : Tests should live alongside code as `*_test.go` files
Applied to files:
cmd/presence_test.gocmd/insights_test.gocmd/apps_test.gocmd/flows_folders_test.go
📚 Learning: 2026-01-15T22:36:58.411Z
Learnt from: CR
Repo: langtind/homeyctl PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-15T22:36:58.411Z
Learning: Applies to internal/client/**/*.go : All API calls must go through `Client.doRequest()` which handles auth headers and error responses
Applied to files:
internal/client/client_test.gointernal/client/client.go
🧬 Code graph analysis (4)
cmd/zones_test.go (1)
cmd/zones.go (1)
KnownZoneIcons(22-56)
internal/client/client_test.go (1)
internal/client/client.go (1)
Client(15-19)
cmd/devices_groups.go (1)
cmd/zones.go (1)
Zone(14-19)
cmd/moods.go (1)
cmd/zones.go (1)
Zone(14-19)
🪛 markdownlint-cli2 (0.18.1)
README.md
423-423: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🔇 Additional comments (86)
.gitignore (1)
4-4: LGTM!Adding the
homeyctlbinary to.gitignoreis correct and follows best practices for Go projects. This ensures the compiled CLI tool isn't accidentally committed to version control.cmd/flows_folders.go (8)
13-18: FlowFolder model is clear and minimal.
20-24: Flows folders command group definition looks good.
26-45: Folder lookup helper is straightforward.
86-104: Get command flow is clean and focused.
106-144: Create command behavior is clear and user-friendly.
146-186: Update command validation and messaging look solid.
188-205: Delete command implementation is straightforward.
207-221: Command wiring and flags look complete.cmd/flows_folders_test.go (8)
7-15: List command existence test looks good.
17-25: Get command existence test looks good.
27-43: Get argument validation coverage looks solid.
45-53: Create command existence test looks good.
55-71: Create argument validation coverage looks solid.
73-81: Delete command existence test looks good.
83-91: Update command existence test looks good.
93-109: Update argument validation coverage looks solid.README.md (1)
1-422: README overhaul is comprehensive and easy to navigate.Also applies to: 427-459
cmd/energy.go (4)
217-267: Yearly report command reads cleanly and consistent with existing reports.
269-291: Delete-all command includes an explicit confirmation gate.
293-315: Currency command output handling is clear.
479-488: Command wiring and flags look consistent.cmd/devices_manage.go (8)
9-38: Rename command flow is straightforward.
40-73: Move command correctly resolves device and zone.
75-107: Set-note command handles update and messaging cleanly.
109-141: Set-icon command handles reset/override paths cleanly.
143-170: Hide command is clear and minimal.
172-197: Unhide command mirrors hide behavior cleanly.
199-216: Delete command is straightforward.
218-226: Command registrations look complete.cmd/zones.go (4)
13-62: Zone model and icon catalog look good.Clear fields and a curated icon list; the updated command description matches the expanded functionality.
64-83: findZone helper is clear and user-friendly.Case-insensitive name matching and explicit error messaging make this easy to use.
115-158: Zones get output handling is solid.Table vs JSON branching is clean, and fetching full details via ID is correct.
186-190: Command wiring looks correct.New subcommands are registered under the zones command as expected.
cmd/ai.go (1)
22-355: AI context updates are clear and safety-focused.The token-scope guidance is explicit and the droptoken pipe format callout is helpful for correctness.
As per coding guidelines, the pipe separator guidance for droptokens is aligned.
internal/client/client.go (7)
91-119: Device update and group endpoints are cleanly wired.Clear endpoints and consistent use of doRequest.
187-203: Zone CRUD client methods look consistent.Matches existing patterns and uses doRequest correctly.
397-417: Flow-folder helpers look good.Endpoints and verb usage align with the rest of the client.
421-465: App lifecycle/settings helpers are consistent.Good coverage of install/uninstall/enable/disable and settings access.
468-541: Users, moods, and dashboards coverage looks solid.Consistent request wiring and clear endpoint structure.
545-575: Presence helpers match established patterns.Straightforward mapping of get/set endpoints.
579-607: Notification & insight deletion helpers look good.URL encoding for insight IDs is handled correctly.
cmd/energy_test.go (1)
1-100: Command existence tests are clear and comprehensive.Good smoke coverage for the new energy subcommands.
cmd/system.go (1)
107-149: Verify the API response format for system name endpoint.The current implementation assumes
GetSystemName()returns a JSON string that can be directly unmarshaled to a string value. If the API returns an object (e.g.,{ "name": "..." }), the parsing will fail. Confirm the actual response shape with the Homey API documentation and update the parsing logic accordingly if needed.cmd/notifications_test.go (1)
1-91: LGTM!The test file follows the project's testing patterns correctly:
- Uses Go's
testingpackage as per guidelines- Validates command existence and naming via
Find()andName()- Validates argument requirements via the
Argsvalidator- Covers all new notification subcommands (send, list, delete, clear, owners)
cmd/insights.go (1)
224-232: LGTM!The new subcommands are correctly wired into the command tree.
cmd/insights_test.go (1)
1-99: LGTM!Comprehensive test coverage for the insights command group:
- Validates existence of all subcommands (list, get, delete, clear)
- Validates argument requirements for commands that require exactly one argument
- Follows the established testing patterns in this codebase
cmd/users_manage.go (2)
14-32: LGTM!The
findUserhelper is well-implemented with both ID and case-insensitive name matching, providing good UX for the CLI.
284-296: LGTM!Commands and flags are correctly wired. The structure follows the project's command organization patterns.
cmd/users_test.go (1)
1-73: LGTM!Tests follow the established patterns and validate command existence and argument requirements.
Consider adding tests for
updateandpresencesubcommands for completeness, though this is optional given the existing coverage pattern.internal/client/client_test.go (3)
10-52: LGTM!Good test structure using httptest for mocking. The test correctly verifies the request path, body content, and error handling.
54-72: LGTM!Error path testing is important - this correctly verifies that non-2xx responses propagate as errors.
1-762: Overall: comprehensive test coverage for the new API client methods.The test suite covers the key API operations with proper HTTP method and path verification. The use of
httptest.NewServeris idiomatic Go. Minor note:w.Write()return values are ignored throughout, but this is acceptable in test code where write failures would surface as test failures anyway.cmd/zones_manage.go (2)
1-51: LGTM!The rename command is well-structured with proper argument validation, zone lookup via
findZone, and conditional output based on whether an icon was also changed.
190-201: LGTM!Commands and flags are properly wired in
init(). The flag registration follows Cobra conventions.cmd/zones_test.go (3)
7-45: LGTM!Good coverage of argument validation for the rename command, testing both invalid (0, 1, 3 args) and valid (2 args) cases.
96-112: LGTM!Proper argument validation test for set-icon command.
123-142: LGTM!Good validation that the KnownZoneIcons list contains expected common icons. The search approach is appropriate for small slices.
cmd/devices_groups.go (3)
13-21: LGTM!Clean type definition with appropriate JSON tags matching the API response structure.
29-92: LGTM!The list command correctly filters for groups by
virtualClass == "group", supports both table and JSON output formats, and returns a flat JSON array as required by the coding guidelines.
312-328: LGTM!Commands and flags are properly wired. Flag names (
--class,--zone,--devices,--add,--remove) are clear and intuitive.cmd/dashboards.go (5)
13-18: LGTM!Clean Dashboard type definition with appropriate fields and JSON tags.
26-45: LGTM!The
findDashboardhelper correctly handles both ID and case-insensitive name lookup.
82-100: LGTM!The get command properly resolves the dashboard first, then fetches full details.
102-156: LGTM!Good flexibility allowing either a simple name-only creation or a full JSON structure. The name from args correctly overrides any name in the JSON file.
212-220: LGTM!Commands properly registered under the dashboards parent command.
cmd/dashboards_test.go (5)
7-15: Clear list subcommand existence test.
17-25: Clear get subcommand existence test.
27-43: Args validator coverage is solid.
45-53: Create subcommand existence check looks good.
55-63: Delete subcommand existence check looks good.cmd/moods_test.go (7)
7-15: List subcommand existence test looks good.
17-25: Get subcommand existence test looks good.
27-43: Args validator coverage for get is solid.
45-53: Create subcommand existence test looks good.
55-63: Delete subcommand existence test looks good.
65-73: Set subcommand existence test looks good.
75-91: Args validator coverage for set is solid.cmd/weather_test.go (2)
7-15: Current subcommand existence test looks good.
17-25: Forecast subcommand existence test looks good.cmd/devices_control.go (2)
27-84: Commands are clear and well-documented.
86-113: Capability guard + command registration look solid.cmd/devices_test.go (1)
7-76: LGTM! Tests properly verify command registration and argument validation.The tests follow good patterns for verifying Cobra command existence and argument validation. Using Go's
testingpackage as per guidelines.Minor note: The ignored errors on lines 19 and 50 (
_, _, _ := devicesCmd.Find(...)) are acceptable here since the earlier tests already verify command existence, but consider using a test helper to reduce duplication if this pattern grows.cmd/weather.go (1)
1-91: Well-structured weather commands with proper output format handling.The implementation correctly follows the command pattern (
homeyctl weather <action>) and supports both JSON and table output viaisTableFormat()andoutputJSON()helpers as per coding guidelines.Good handling of empty forecast data (lines 68-71) and proper error propagation throughout.
cmd/apps_test.go (1)
1-113: LGTM! Comprehensive test coverage for apps command surface.Tests properly verify all subcommand registrations and argument validation for the install command. Follows the same well-structured pattern as the devices tests and uses Go's
testingpackage as per guidelines.cmd/apps_manage.go (1)
225-241: LGTM! Commands are properly wired with appropriate flags.The
init()function correctly registers all subcommands and their flags under the apps command hierarchy.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
| if isTableFormat() { | ||
| var app struct { | ||
| ID string `json:"id"` | ||
| Name string `json:"name"` | ||
| Version string `json:"version"` | ||
| } | ||
| json.Unmarshal(result, &app) | ||
| fmt.Printf("Installed app: %s v%s\n", app.Name, app.Version) | ||
| return nil | ||
| } |
There was a problem hiding this comment.
Handle JSON unmarshal error for better user feedback.
The json.Unmarshal error on line 37 is ignored. If unmarshalling fails, app.Name and app.Version will be empty strings, resulting in confusing output like "Installed app: v".
Proposed fix
if isTableFormat() {
var app struct {
ID string `json:"id"`
Name string `json:"name"`
Version string `json:"version"`
}
- json.Unmarshal(result, &app)
- fmt.Printf("Installed app: %s v%s\n", app.Name, app.Version)
+ if err := json.Unmarshal(result, &app); err != nil {
+ return fmt.Errorf("failed to parse install response: %w", err)
+ }
+ fmt.Printf("Installed app: %s v%s\n", app.Name, app.Version)
return nil
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if isTableFormat() { | |
| var app struct { | |
| ID string `json:"id"` | |
| Name string `json:"name"` | |
| Version string `json:"version"` | |
| } | |
| json.Unmarshal(result, &app) | |
| fmt.Printf("Installed app: %s v%s\n", app.Name, app.Version) | |
| return nil | |
| } | |
| if isTableFormat() { | |
| var app struct { | |
| ID string `json:"id"` | |
| Name string `json:"name"` | |
| Version string `json:"version"` | |
| } | |
| if err := json.Unmarshal(result, &app); err != nil { | |
| return fmt.Errorf("failed to parse install response: %w", err) | |
| } | |
| fmt.Printf("Installed app: %s v%s\n", app.Name, app.Version) | |
| return nil | |
| } |
🤖 Prompt for AI Agents
In `@cmd/apps_manage.go` around lines 31 - 40, The json.Unmarshal call in the
branch that checks isTableFormat() ignores its returned error which can lead to
empty app.Name/app.Version and confusing output; update the code around the
json.Unmarshal(result, &app) call to capture the error, and if non-nil return or
log a user-friendly error (including the unmarshalling error and possibly the
raw result) instead of proceeding to fmt.Printf("Installed app: %s v%s\n",
app.Name, app.Version); reference the isTableFormat() branch, the local variable
app, the json.Unmarshal call, and the fmt.Printf so you validate the unmarshal
before printing and return an appropriate error or message.
| var appsUpdateCmd = &cobra.Command{ | ||
| Use: "update <app>", | ||
| Short: "Update app settings", | ||
| Long: `Update app settings like autoupdate. | ||
|
|
||
| Examples: | ||
| homeyctl apps update com.fibaro --autoupdate=true | ||
| homeyctl apps update com.fibaro --autoupdate=false`, | ||
| Args: cobra.ExactArgs(1), | ||
| RunE: func(cmd *cobra.Command, args []string) error { | ||
| app, err := findApp(args[0]) | ||
| if err != nil { | ||
| return err | ||
| } | ||
|
|
||
| autoupdate, _ := cmd.Flags().GetBool("autoupdate") | ||
|
|
||
| updates := map[string]interface{}{ | ||
| "autoupdate": autoupdate, | ||
| } | ||
|
|
||
| if err := apiClient.UpdateApp(app.ID, updates); err != nil { | ||
| return err | ||
| } | ||
|
|
||
| fmt.Printf("Updated app: %s (autoupdate: %v)\n", app.Name, autoupdate) | ||
| return nil | ||
| }, | ||
| } |
There was a problem hiding this comment.
apps update command unconditionally sets autoupdate, even when flag is not provided.
The command always reads --autoupdate (defaulting to true) and sends it in the update payload. This means running homeyctl apps update myapp without any flag will set autoupdate=true, which may not be the user's intent.
Consider checking if the flag was explicitly changed, or requiring the flag to be passed.
Proposed fix using Changed() check
RunE: func(cmd *cobra.Command, args []string) error {
app, err := findApp(args[0])
if err != nil {
return err
}
- autoupdate, _ := cmd.Flags().GetBool("autoupdate")
-
- updates := map[string]interface{}{
- "autoupdate": autoupdate,
+ updates := map[string]interface{}{}
+
+ if cmd.Flags().Changed("autoupdate") {
+ autoupdate, _ := cmd.Flags().GetBool("autoupdate")
+ updates["autoupdate"] = autoupdate
+ }
+
+ if len(updates) == 0 {
+ return fmt.Errorf("no update flags provided")
}
if err := apiClient.UpdateApp(app.ID, updates); err != nil {
return err
}
- fmt.Printf("Updated app: %s (autoupdate: %v)\n", app.Name, autoupdate)
+ fmt.Printf("Updated app: %s\n", app.Name)
return nil
},🤖 Prompt for AI Agents
In `@cmd/apps_manage.go` around lines 104 - 132, The apps update command currently
always reads the "autoupdate" boolean flag and sends it in the updates payload,
causing unintended changes when the flag wasn't provided; modify the RunE for
appsUpdateCmd to call cmd.Flags().Changed("autoupdate") and only include the
"autoupdate" key in the updates map if Changed(...) is true (or return a
user-facing error when no flags were provided), so apiClient.UpdateApp receives
only explicitly set fields; update references in RunE where
cmd.Flags().GetBool("autoupdate"), the updates map, and the apiClient.UpdateApp
call accordingly.
| var dashboardsListCmd = &cobra.Command{ | ||
| Use: "list", | ||
| Short: "List all dashboards", | ||
| RunE: func(cmd *cobra.Command, args []string) error { | ||
| data, err := apiClient.GetDashboards() | ||
| if err != nil { | ||
| return err | ||
| } | ||
|
|
||
| if isTableFormat() { | ||
| var dashboards map[string]Dashboard | ||
| if err := json.Unmarshal(data, &dashboards); err != nil { | ||
| return fmt.Errorf("failed to parse dashboards: %w", err) | ||
| } | ||
|
|
||
| if len(dashboards) == 0 { | ||
| fmt.Println("No dashboards found.") | ||
| return nil | ||
| } | ||
|
|
||
| w := tabwriter.NewWriter(os.Stdout, 0, 0, 2, ' ', 0) | ||
| fmt.Fprintln(w, "NAME\tCOLUMNS\tID") | ||
| fmt.Fprintln(w, "----\t-------\t--") | ||
| for _, d := range dashboards { | ||
| fmt.Fprintf(w, "%s\t%d\t%s\n", d.Name, len(d.Columns), d.ID) | ||
| } | ||
| w.Flush() | ||
| return nil | ||
| } | ||
|
|
||
| outputJSON(data) | ||
| return nil | ||
| }, | ||
| } |
There was a problem hiding this comment.
JSON output should return a flat array, not a map.
Per the coding guidelines, "All list commands must return flat JSON arrays for easy parsing." Line 77 outputs the raw API response (a map keyed by dashboard ID), but should convert it to an array like the table format does.
🔧 Proposed fix
if isTableFormat() {
// ... existing table code ...
}
- outputJSON(data)
+ var dashboards map[string]Dashboard
+ if err := json.Unmarshal(data, &dashboards); err != nil {
+ return fmt.Errorf("failed to parse dashboards: %w", err)
+ }
+
+ // Convert map to flat array for consistent JSON output
+ dashboardList := make([]Dashboard, 0, len(dashboards))
+ for _, d := range dashboards {
+ dashboardList = append(dashboardList, d)
+ }
+
+ out, _ := json.MarshalIndent(dashboardList, "", " ")
+ fmt.Println(string(out))
return nilBased on coding guidelines: "All list commands must return flat JSON arrays for easy parsing."
🤖 Prompt for AI Agents
In `@cmd/dashboards.go` around lines 47 - 80, The JSON output currently returns
the raw map from apiClient.GetDashboards() instead of a flat array; update the
RunE of dashboardsListCmd to unmarshal the API response into a
map[string]Dashboard (as already done for table output), convert that map's
values into a []Dashboard slice (preserve iteration order if needed or sort by
key/name), then marshal that slice and pass the marshaled bytes to outputJSON
(or call outputJSON with the slice if it accepts interface{}), ensuring the JSON
path uses the same Dashboard type and error handling as the existing
json.Unmarshal and fmt.Errorf patterns.
| // parseValue converts a string value to the appropriate type (bool, number, or string) | ||
| func parseValue(valueStr string) interface{} { | ||
| if valueStr == "true" { | ||
| return true | ||
| } | ||
| if valueStr == "false" { | ||
| return false | ||
| } | ||
|
|
||
| // Try as number | ||
| var num float64 | ||
| if _, err := fmt.Sscanf(valueStr, "%f", &num); err == nil { | ||
| return num | ||
| } | ||
|
|
||
| return valueStr | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, check if the file exists and contains the parseValue function
find . -name "devices_control.go" -type fRepository: langtind/homeyctl
Length of output: 85
🏁 Script executed:
# Read the devices_control.go file to see the current state
cat -n cmd/devices_control.go | head -50Repository: langtind/homeyctl
Length of output: 1490
🏁 Script executed:
# Test fmt.Sscanf behavior with partial numeric input to verify the claim
python3 << 'EOF'
import subprocess
# Create a test Go program to verify fmt.Sscanf behavior
test_code = '''
package main
import (
"fmt"
)
func main() {
// Test partial numeric input
var num float64
n, err := fmt.Sscanf("12abc", "%f", &num)
fmt.Printf("Partial input '12abc': n=%d, err=%v, num=%v\n", n, err, num)
// Test valid numeric input
n, err = fmt.Sscanf("12.5", "%f", &num)
fmt.Printf("Valid input '12.5': n=%d, err=%v, num=%v\n", n, err, num)
// Test strconv behavior for comparison
f := strconv.ParseFloat("12abc", 64)
fmt.Printf("strconv.ParseFloat('12abc'): %v\n", f)
f = strconv.ParseFloat("12.5", 64)
fmt.Printf("strconv.ParseFloat('12.5'): %v\n", f)
}
'''
# Write the test code to a temp file
with open('/tmp/test_sscanf.go', 'w') as f:
f.write(test_code)
# Run it
result = subprocess.run(['go', 'run', '/tmp/test_sscanf.go'], capture_output=True, text=True, cwd='.')
print("STDOUT:")
print(result.stdout)
if result.stderr:
print("STDERR:")
print(result.stderr)
EOFRepository: langtind/homeyctl
Length of output: 147
🏁 Script executed:
# Check what's in the import section of devices_control.go
head -20 cmd/devices_control.goRepository: langtind/homeyctl
Length of output: 437
🏁 Script executed:
# Test fmt.Sscanf partial parsing behavior with a simpler approach
cat > /tmp/test_parse.go << 'EOF'
package main
import (
"fmt"
"strconv"
)
func main() {
// Test fmt.Sscanf with partial input
var num float64
n, err := fmt.Sscanf("12abc", "%f", &num)
if err == nil {
fmt.Printf("fmt.Sscanf('12abc'): accepted, parsed value=%v, n=%d\n", num, n)
} else {
fmt.Printf("fmt.Sscanf('12abc'): error=%v\n", err)
}
// Test strconv.ParseFloat with same input
parsed, err := strconv.ParseFloat("12abc", 64)
if err == nil {
fmt.Printf("strconv.ParseFloat('12abc'): accepted, value=%v\n", parsed)
} else {
fmt.Printf("strconv.ParseFloat('12abc'): error=%v (correctly rejects)\n", err)
}
}
EOF
go run /tmp/test_parse.goRepository: langtind/homeyctl
Length of output: 337
🏁 Script executed:
# Check the coding guidelines to see if there are any specifications about numeric parsing
grep -r "strconv\|Sscanf\|parse" . --include="*.md" 2>/dev/null | head -20Repository: langtind/homeyctl
Length of output: 223
🏁 Script executed:
# Check if strconv is used elsewhere in the codebase
rg "strconv" --type goRepository: langtind/homeyctl
Length of output: 43
🏁 Script executed:
# Check if fmt.Sscanf is used elsewhere in the codebase
rg "fmt\.Sscanf" --type goRepository: langtind/homeyctl
Length of output: 556
Use strconv.ParseBool and strconv.ParseFloat for safer numeric parsing.
fmt.Sscanf with format strings accepts partial numeric input (e.g., "12abc" → 12), silently coercing invalid inputs. strconv functions validate the entire string and reject malformed values.
💡 Safer parsing
@@
-import (
- "fmt"
-
- "github.com/spf13/cobra"
-)
+import (
+ "fmt"
+ "strconv"
+
+ "github.com/spf13/cobra"
+)
@@
func parseValue(valueStr string) interface{} {
- if valueStr == "true" {
- return true
- }
- if valueStr == "false" {
- return false
- }
-
- // Try as number
- var num float64
- if _, err := fmt.Sscanf(valueStr, "%f", &num); err == nil {
- return num
- }
+ if b, err := strconv.ParseBool(valueStr); err == nil {
+ return b
+ }
+ if num, err := strconv.ParseFloat(valueStr, 64); err == nil {
+ return num
+ }
return valueStr
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // parseValue converts a string value to the appropriate type (bool, number, or string) | |
| func parseValue(valueStr string) interface{} { | |
| if valueStr == "true" { | |
| return true | |
| } | |
| if valueStr == "false" { | |
| return false | |
| } | |
| // Try as number | |
| var num float64 | |
| if _, err := fmt.Sscanf(valueStr, "%f", &num); err == nil { | |
| return num | |
| } | |
| return valueStr | |
| } | |
| // parseValue converts a string value to the appropriate type (bool, number, or string) | |
| func parseValue(valueStr string) interface{} { | |
| if b, err := strconv.ParseBool(valueStr); err == nil { | |
| return b | |
| } | |
| if num, err := strconv.ParseFloat(valueStr, 64); err == nil { | |
| return num | |
| } | |
| return valueStr | |
| } |
🤖 Prompt for AI Agents
In `@cmd/devices_control.go` around lines 9 - 25, The parseValue function should
use strconv.ParseBool for boolean detection and strconv.ParseFloat (with 64-bit)
to validate numeric input instead of fmt.Sscanf; update parseValue to first
attempt strconv.ParseBool(valueStr) and return the bool if no error, then
attempt strconv.ParseFloat(valueStr, 64) and return the float64 if no error,
otherwise fall back to returning the original string — this ensures the entire
string is validated and rejects partial/malformed numeric inputs.
| var flowsFoldersListCmd = &cobra.Command{ | ||
| Use: "list", | ||
| Short: "List all flow folders", | ||
| RunE: func(cmd *cobra.Command, args []string) error { | ||
| data, err := apiClient.GetFlowFolders() | ||
| if err != nil { | ||
| return err | ||
| } | ||
|
|
||
| if isTableFormat() { | ||
| var folders map[string]FlowFolder | ||
| if err := json.Unmarshal(data, &folders); err != nil { | ||
| return fmt.Errorf("failed to parse flow folders: %w", err) | ||
| } | ||
|
|
||
| if len(folders) == 0 { | ||
| fmt.Println("No flow folders found.") | ||
| return nil | ||
| } | ||
|
|
||
| w := tabwriter.NewWriter(os.Stdout, 0, 0, 2, ' ', 0) | ||
| fmt.Fprintln(w, "NAME\tPARENT\tID") | ||
| fmt.Fprintln(w, "----\t------\t--") | ||
| for _, f := range folders { | ||
| parent := f.Parent | ||
| if parent == "" { | ||
| parent = "(root)" | ||
| } | ||
| fmt.Fprintf(w, "%s\t%s\t%s\n", f.Name, parent, f.ID) | ||
| } | ||
| w.Flush() | ||
| return nil | ||
| } | ||
|
|
||
| outputJSON(data) | ||
| return nil | ||
| }, |
There was a problem hiding this comment.
Return a flat JSON array for flows folders list.
The JSON path currently forwards the map payload, which serializes as an object keyed by IDs; list output should be a flat array for scripts. As per coding guidelines, list commands must return flat JSON arrays.
🛠️ Proposed fix
- if isTableFormat() {
- var folders map[string]FlowFolder
- if err := json.Unmarshal(data, &folders); err != nil {
- return fmt.Errorf("failed to parse flow folders: %w", err)
- }
-
- if len(folders) == 0 {
- fmt.Println("No flow folders found.")
- return nil
- }
-
- w := tabwriter.NewWriter(os.Stdout, 0, 0, 2, ' ', 0)
- fmt.Fprintln(w, "NAME\tPARENT\tID")
- fmt.Fprintln(w, "----\t------\t--")
- for _, f := range folders {
- parent := f.Parent
- if parent == "" {
- parent = "(root)"
- }
- fmt.Fprintf(w, "%s\t%s\t%s\n", f.Name, parent, f.ID)
- }
- w.Flush()
- return nil
- }
-
- outputJSON(data)
- return nil
+ var folders map[string]FlowFolder
+ if err := json.Unmarshal(data, &folders); err != nil {
+ return fmt.Errorf("failed to parse flow folders: %w", err)
+ }
+
+ if isTableFormat() {
+ if len(folders) == 0 {
+ fmt.Println("No flow folders found.")
+ return nil
+ }
+
+ w := tabwriter.NewWriter(os.Stdout, 0, 0, 2, ' ', 0)
+ fmt.Fprintln(w, "NAME\tPARENT\tID")
+ fmt.Fprintln(w, "----\t------\t--")
+ for _, f := range folders {
+ parent := f.Parent
+ if parent == "" {
+ parent = "(root)"
+ }
+ fmt.Fprintf(w, "%s\t%s\t%s\n", f.Name, parent, f.ID)
+ }
+ w.Flush()
+ return nil
+ }
+
+ list := make([]FlowFolder, 0, len(folders))
+ for _, f := range folders {
+ list = append(list, f)
+ }
+ out, err := json.Marshal(list)
+ if err != nil {
+ return fmt.Errorf("failed to serialize flow folders: %w", err)
+ }
+ outputJSON(out)
+ return nil🤖 Prompt for AI Agents
In `@cmd/flows_folders.go` around lines 47 - 83, flowsFoldersListCmd currently
returns the raw map payload for JSON output which serializes as an object keyed
by IDs; unmarshal the response from apiClient.GetFlowFolders() into
map[string]FlowFolder (same as in the table branch), convert that map into a
flat []FlowFolder slice (preserving order if needed), then marshal or pass that
slice to outputJSON so the command emits a flat JSON array; update the JSON-path
code path in flowsFoldersListCmd (and reuse the existing json.Unmarshal logic)
instead of calling outputJSON(data) with the original bytes.
| if nameOrID == "me" { | ||
| data, err := apiClient.GetUserMe() | ||
| if err != nil { | ||
| return err | ||
| } | ||
| var u struct { | ||
| ID string `json:"id"` | ||
| Name string `json:"name"` | ||
| } | ||
| json.Unmarshal(data, &u) | ||
| userID = u.ID | ||
| userName = u.Name | ||
| } else { | ||
| user, err := findUser(nameOrID) | ||
| if err != nil { | ||
| return err | ||
| } | ||
| userID = user.ID | ||
| userName = user.Name | ||
| } | ||
|
|
||
| data, err := apiClient.GetAsleep(userID) | ||
| if err != nil { | ||
| return err | ||
| } | ||
|
|
||
| var asleep struct { | ||
| Value bool `json:"value"` | ||
| } | ||
| json.Unmarshal(data, &asleep) | ||
|
|
||
| if isTableFormat() { | ||
| status := "awake" | ||
| if asleep.Value { | ||
| status = "asleep" | ||
| } | ||
| fmt.Printf("%s is %s\n", userName, status) | ||
| return nil | ||
| } | ||
|
|
||
| outputJSON(data) | ||
| return nil | ||
| }, |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n cmd/presence.go | sed -n '150,210p'Repository: langtind/homeyctl
Length of output: 1641
🏁 Script executed:
# Search for other json.Unmarshal patterns in the codebase to check consistency
rg "json\.Unmarshal" -A 1 -B 1 --no-headingRepository: langtind/homeyctl
Length of output: 14155
🏁 Script executed:
# Check if there's a specific error handling pattern used in other commands
rg "json\.Unmarshal" -A 2 --type go | head -50Repository: langtind/homeyctl
Length of output: 2110
Add error handling for JSON unmarshal operations in presence asleep get.
The unmarshaling of GetUserMe and GetAsleep responses (lines 170 and 190) lacks error checks, allowing silent parsing failures to corrupt output.
Proposed fix
@@
- json.Unmarshal(data, &u)
+ if err := json.Unmarshal(data, &u); err != nil {
+ return fmt.Errorf("failed to parse user info: %w", err)
+ }
@@
- json.Unmarshal(data, &asleep)
+ if err := json.Unmarshal(data, &asleep); err != nil {
+ return fmt.Errorf("failed to parse sleep status: %w", err)
+ }🤖 Prompt for AI Agents
In `@cmd/presence.go` around lines 161 - 203, The json.Unmarshal calls for the
GetUserMe and GetAsleep responses are missing error handling; update the block
that handles nameOrID == "me" to capture and check the error returned by
json.Unmarshal(data, &u) (where u is the temporary struct used for ID/Name) and
return a descriptive error if unmarshalling fails, and similarly check the error
returned by json.Unmarshal(data, &asleep) (where asleep is the temporary struct
used for Value) before using asleep.Value; ensure both error returns surface
useful context (e.g., which API response failed) and keep existing control flow
that prints table format or calls outputJSON(data).
| validRoles := []string{"guest", "user", "manager"} | ||
| isValid := false | ||
| for _, r := range validRoles { | ||
| if r == role { | ||
| isValid = true | ||
| break | ||
| } | ||
| } | ||
| if !isValid { | ||
| return fmt.Errorf("invalid role: %s (valid: guest, user, manager)", role) | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cd $(git rev-parse --show-toplevel) && rg -n -C5 'owner' --type=go cmd/users_manage.goRepository: langtind/homeyctl
Length of output: 492
🏁 Script executed:
cd $(git rev-parse --show-toplevel) && sed -n '280,300p' cmd/users_manage.goRepository: langtind/homeyctl
Length of output: 549
🏁 Script executed:
cd $(git rev-parse --show-toplevel) && rg -n 'validRoles|isValid' --type=go cmd/users_manage.goRepository: langtind/homeyctl
Length of output: 220
🏁 Script executed:
cd $(git rev-parse --show-toplevel) && rg -n -A30 'var usersUpdateCmd' --type=go cmd/users_manage.go | head -60Repository: langtind/homeyctl
Length of output: 1003
🏁 Script executed:
cd $(git rev-parse --show-toplevel) && sed -n '150,210p' cmd/users_manage.goRepository: langtind/homeyctl
Length of output: 1386
Add role validation to the update command and clarify the supported roles across both commands.
The create command validates roles against ["guest", "user", "manager"], while the update command accepts any role string without validation (lines 173-174) yet advertises "owner" as valid in its flag help (line 292). This creates inconsistency:
- The update command performs no validation—any string is accepted and sent to the API
- The create command restricts to three roles while update mentions a fourth
- If "owner" is legitimately promotable, the create command should also document this constraint; otherwise, the update help text should match
Either add role validation to the update command matching the create logic, or verify that all advertised roles (including "owner") are accepted by the API and align the help texts accordingly.
🤖 Prompt for AI Agents
In `@cmd/users_manage.go` around lines 125 - 135, The update path currently
accepts any role string while the create path restricts roles via the validRoles
check (variables validRoles/isValid and the role variable); add the same
validation logic to the update command handler (reuse validRoles and the isValid
loop) and return the same formatted error if invalid, and then make the flag
help text consistent (remove or add "owner" so both create and update advertise
the same supported roles). Ensure you update the update command's role parsing
code (the block around where role is read at the update handler) and the flag
help string that currently mentions "owner" so both commands match.
| for _, u := range users { | ||
| // Get presence status | ||
| presentData, _ := apiClient.GetPresent(u.ID) | ||
| asleepData, _ := apiClient.GetAsleep(u.ID) | ||
|
|
||
| var present, asleep struct { | ||
| Value bool `json:"value"` | ||
| } | ||
| json.Unmarshal(presentData, &present) | ||
| json.Unmarshal(asleepData, &asleep) | ||
|
|
||
| presentStr := "no" | ||
| if present.Value { | ||
| presentStr = "yes" | ||
| } | ||
| asleepStr := "no" | ||
| if asleep.Value { | ||
| asleepStr = "yes" | ||
| } | ||
|
|
||
| fmt.Fprintf(w, "%s\t%s\t%s\t%s\n", u.Name, presentStr, asleepStr, u.ID) | ||
| } | ||
| w.Flush() | ||
| return nil |
There was a problem hiding this comment.
Silent error handling may hide API failures.
Errors from GetPresent() and GetAsleep() are silently ignored (lines 235-236), as are json.Unmarshal errors (lines 241-242). If an API call fails, the user will appear as "not present" / "not asleep" without any indication of the failure.
Consider logging a warning or showing "unknown" status when the API call fails, rather than defaulting to false.
💡 Suggested approach
for _, u := range users {
- // Get presence status
- presentData, _ := apiClient.GetPresent(u.ID)
- asleepData, _ := apiClient.GetAsleep(u.ID)
+ presentData, presentErr := apiClient.GetPresent(u.ID)
+ asleepData, asleepErr := apiClient.GetAsleep(u.ID)
var present, asleep struct {
Value bool `json:"value"`
}
json.Unmarshal(presentData, &present)
json.Unmarshal(asleepData, &asleep)
- presentStr := "no"
- if present.Value {
+ presentStr := "?"
+ if presentErr == nil {
+ presentStr = "no"
+ if present.Value {
+ presentStr = "yes"
+ }
+ }
+ asleepStr := "?"
+ if asleepErr == nil {
+ asleepStr = "no"
+ if asleep.Value {
+ asleepStr = "yes"
+ }
}
- // ... similar for asleepStr🤖 Prompt for AI Agents
In `@cmd/users_manage.go` around lines 233 - 256, The loop over users currently
ignores errors from apiClient.GetPresent/GetAsleep and json.Unmarshal which can
hide API failures; update the code in the users iteration (the block using
apiClient.GetPresent, apiClient.GetAsleep, the present/asleep structs and
json.Unmarshal) to check and handle both API and unmarshal errors: if
GetPresent/GetAsleep returns an error or unmarshalling fails, log a warning (or
write to stderr) and set the corresponding presentStr/asleepStr to "unknown"
instead of defaulting to "no", otherwise use the unmarshalled Value to choose
"yes"/"no"; keep the final fmt.Fprintf(w, ...) and w.Flush() but ensure error
paths are handled before formatting.
| func (c *Client) GetSystemName() (json.RawMessage, error) { | ||
| return c.doRequest("GET", "/api/manager/system/name", nil) | ||
| } | ||
|
|
||
| func (c *Client) SetSystemName(name string) error { | ||
| body := map[string]interface{}{"name": name} | ||
| _, err := c.doRequest("PUT", "/api/manager/system/name", body) | ||
| return err | ||
| } | ||
|
|
||
| // Weather | ||
|
|
||
| func (c *Client) GetWeather() (json.RawMessage, error) { | ||
| return c.doRequest("GET", "/api/manager/weather/weather", nil) | ||
| } | ||
|
|
||
| func (c *Client) GetWeatherForecast() (json.RawMessage, error) { | ||
| return c.doRequest("GET", "/api/manager/weather/forecast/hourly", nil) | ||
| } | ||
|
|
||
| // Energy (extended) | ||
|
|
||
| func (c *Client) GetEnergyReportYear(year string) (json.RawMessage, error) { | ||
| return c.doRequest("GET", "/api/manager/energy/report/year?year="+year, nil) | ||
| } | ||
|
|
||
| func (c *Client) DeleteEnergyReports() error { | ||
| _, err := c.doRequest("DELETE", "/api/manager/energy/reports", nil) | ||
| return err | ||
| } | ||
|
|
||
| func (c *Client) GetEnergyCurrency() (json.RawMessage, error) { | ||
| return c.doRequest("GET", "/api/manager/energy/currency", nil) | ||
| } |
There was a problem hiding this comment.
Guard empty year parameter in GetEnergyReportYear.
Other report helpers only add query params when provided; always sending ?year= could break “current year” calls if the year is optional. Consider mirroring the conditional pattern and escaping the param.
🐛 Suggested fix
func (c *Client) GetEnergyReportYear(year string) (json.RawMessage, error) {
- return c.doRequest("GET", "/api/manager/energy/report/year?year="+year, nil)
+ path := "/api/manager/energy/report/year"
+ if year != "" {
+ path += "?year=" + url.QueryEscape(year)
+ }
+ return c.doRequest("GET", path, nil)
}🤖 Prompt for AI Agents
In `@internal/client/client.go` around lines 611 - 644, GetEnergyReportYear
currently always appends "?year="+year which sends an empty query when year is
empty; change GetEnergyReportYear to build the request path conditionally: if
year is non-empty, append "?year="+url.QueryEscape(year) to
"/api/manager/energy/report/year", otherwise call doRequest with just
"/api/manager/energy/report/year". Reference the GetEnergyReportYear method and
use url.QueryEscape to properly escape the year parameter.
| ``` | ||
| CORRECT: "homey:device:abc123|measure_temperature" | ||
| WRONG: "homey:device:abc123:measure_temperature" | ||
| ``` |
There was a problem hiding this comment.
Specify a language for the droptoken code fence.
Markdownlint flags fenced code blocks without a language tag; adding one also improves readability.
🛠️ Proposed fix
-```
+```text
CORRECT: "homey:device:abc123|measure_temperature"
WRONG: "homey:device:abc123:measure_temperature"
-```
+```🧰 Tools
🪛 markdownlint-cli2 (0.18.1)
423-423: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
In `@README.md` around lines 423 - 426, Update the fenced code block that shows
the CORRECT/WRONG droptoken examples to include a language tag (use "text") so
markdownlint stops flagging it; locate the snippet containing the strings
"CORRECT: \"homey:device:abc123|measure_temperature\"" and "WRONG:
\"homey:device:abc123:measure_temperature\"" and change the opening fence from
``` to ```text while keeping the closing fence as ``` to improve linting and
readability.
1 participant
Summary
This PR adds comprehensive support for the Homey API, implementing 46+ new commands across 11 categories.
New Features
Device & Zone Management
Smart Home
Flow Management
App Management
Extended Features
Documentation
homeyctl ai) organized by access levelTests
Test plan
--helpSummary by CodeRabbit
New Features
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.