Packages by rustyeddy · Pull Request #3 · rustyeddy/gardener

rustyeddy · 2025-11-25T19:08:15Z

Have actions pipeline generate binaries for different OS/Architecture combos.

Copilot

Pull request overview

This PR configures GitHub Actions to build cross-platform binaries for multiple operating systems and architectures, while also making several code changes including dependency updates, message topic reorganization, and device initialization refactoring.

Adds a comprehensive GitHub Actions workflow matrix to build binaries for Linux (amd64, ARM variants), macOS (Intel and Apple Silicon), and Windows
Updates dependencies (devices v0.0.1→v0.0.3, otto v0.0.9→v0.0.11) and removes local replace directives to use published versions
Refactors device initialization to store devices as struct fields and changes MQTT topic naming scheme with "d/" and "c/" prefixes

Reviewed changes

Copilot reviewed 4 out of 5 changed files in this pull request and generated 9 comments.

Show a summary per file

File	Description
.github/workflows/go.yml	Adds multi-architecture build matrix for Linux (ARM variants), macOS, and Windows with artifact uploads
go.mod	Updates dependency versions and comments out local replace directives for remote packages
go.sum	Adds checksums for updated dependency versions
main.go	Renames log file and removes goroutine wrapper from gardener.Start() call
gardener.go	Refactors device initialization to use struct fields, updates MQTT topics with prefixes, adds centralized message handler

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2025-12-22T23:28:18Z

+          cache: true
+
+      - name: Prepare dist directory
+        run: mkdir -p dist


The mkdir command syntax is Unix-specific and will fail on Windows runners. While 'mkdir dist' might work on Windows PowerShell, the '-p' flag is bash-specific. For cross-platform compatibility with all matrix builds, specify 'shell: bash' or use a platform-independent approach. Since the workflow already needs bash for the build step, add 'shell: bash' here as well.

Suggested change

run: mkdir -p dist

run: mkdir -p dist

shell: bash

Copilot · 2025-12-22T23:28:18Z

+		fmt.Println("Got soil: ")
+
+	case "env":
+		fmt.Println("Got env: ")
+
+	case "on", "off":
+		fmt.Println("Got a button")
+
+	default:
+		slog.Error("unknown msg type", "topic", msg.Topic, "msg", msg)
+	}
+	fmt.Printf("msg: %#v\n", msg)


The MsgHandler function uses fmt.Println for debugging output (lines 189, 192, 195, 200) instead of the structured logging (slog) used throughout the rest of the codebase. For consistency and proper log level control, these should use slog.Debug or slog.Info. Additionally, the messages are incomplete (e.g., "Got soil: " with no value), which suggests these are debugging statements that may have been left in accidentally.

Suggested change

fmt.Println("Got soil: ")

case "env":

fmt.Println("Got env: ")

case "on", "off":

fmt.Println("Got a button")

default:

slog.Error("unknown msg type", "topic", msg.Topic, "msg", msg)

}

fmt.Printf("msg: %#v\n", msg)

slog.Debug("received soil message", "topic", msg.Topic, "value", msg.Data)

case "env":

slog.Debug("received env message", "topic", msg.Topic, "value", msg.Data)

case "on", "off":

slog.Debug("received button message", "topic", msg.Topic, "value", msg.Data)

default:

slog.Error("unknown msg type", "topic", msg.Topic, "msg", msg)

}

slog.Debug("raw MQTT message", "msg", msg)

Copilot · 2025-12-22T23:28:19Z

 	gardener := &Gardener{}
 	gardener.Init()
-	go gardener.Start()
+	gardener.Start()


Removing the goroutine from gardener.Start() changes the application's behavior significantly. The Start() method now blocks the main thread, which means signal handling won't work until Start() returns. This breaks graceful shutdown because the signal handler goroutine at line 72-76 won't be able to process signals while Start() is blocking.

Looking at the Start() method in gardener.go (lines 165-182), it doesn't appear to have a blocking loop, so this might work differently than expected. However, if Start() is intended to be blocking (e.g., if it starts servers or waits for connections), the signal handler needs to be set up before calling Start(), which it already is. But if Start() returns immediately, the program will proceed to wait on gardener.Done at line 78 without anything blocking.

Copilot · 2025-12-22T23:28:19Z


 func (g *Gardener) initDisplay() {
-	display, err := oled.New("lcd", 0x27, 1)
+	display, err := oled.New("c/lcd", 0x27, 1)


The display variable is assigned but never stored in the struct field g.display (line 32). This means the display device cannot be accessed later by other methods of the Gardener struct. The pump device is similarly created but assigned to g.pump, while display should be assigned to g.display for consistency with other device initialization methods.

Copilot · 2025-12-22T23:28:19Z

+	topics := []string{"soil", "env", "on", "off", "pump", "display"}
+	for _, topic := range topics {
+		g.Subscribe(topic, g.MsgHandler)
+	}


The message topic subscriptions use base topic names ("soil", "env", "on", "off", "pump", "display"), but the publishing code uses prefixed topics like "d/soil", "d/env", "d/on", "d/off" (device topics) and "c/pump", "c/lcd" (control topics). This mismatch means published messages won't match the subscribed topics, breaking the message flow. The subscriptions should use the same topic prefixes as the publishers, or the publishers should use unprefixed topics.

Copilot · 2025-12-22T23:28:19Z

+	g.pump, err = relay.New("pump", pinmap["pump"])
+	if err != nil {
+		panic(err)
+	}


The pump device is initialized but never added to the DeviceManager, unlike all other devices (soil, env, on, off buttons, display). This inconsistency means the pump won't be tracked or managed like other devices. Add g.DeviceManager.Add(g.pump) after line 150, similar to how other devices are registered.

Suggested change

}

}

g.DeviceManager.Add(g.pump)

Copilot · 2025-12-22T23:28:19Z

+          if [ "$GOOS" = "windows" ]; then EXT=".exe"; fi
+          OUT="dist/${BIN_NAME}-${{ matrix.build.name }}${EXT}"
+          echo "Building $OUT (GOOS=$GOOS GOARCH=$GOARCH GOARM=$GOARM)"
+          go build -trimpath -ldflags "-s -w" -o "$OUT" ./...


The build command uses 'go build ... ./...' which builds all packages in the module and its subdirectories. However, the intent appears to be building a single binary from the main package. This should be 'go build -trimpath -ldflags "-s -w" -o "$OUT"' (without ./...) or 'go build -trimpath -ldflags "-s -w" -o "$OUT" .' to build only the main package. Building with ./... may produce unexpected results or fail if there are multiple main packages.

Suggested change

go build -trimpath -ldflags "-s -w" -o "$OUT" ./...

go build -trimpath -ldflags "-s -w" -o "$OUT" .

Copilot · 2025-12-22T23:28:20Z

+      - name: Build binary
+        env:
+          GOOS: ${{ matrix.build.goos }}
+          GOARCH: ${{ matrix.build.goarch }}
+          GOARM: ${{ matrix.build.goarm }}
+        run: |
+          set -e
+          BIN_NAME=garden-station
+          EXT=""
+          if [ "$GOOS" = "windows" ]; then EXT=".exe"; fi
+          OUT="dist/${BIN_NAME}-${{ matrix.build.name }}${EXT}"
+          echo "Building $OUT (GOOS=$GOOS GOARCH=$GOARCH GOARM=$GOARM)"
+          go build -trimpath -ldflags "-s -w" -o "$OUT" ./...
+          ls -l "$OUT"


The shell script uses bash-specific syntax ('set -e', conditional with 'if [ ... ]; then'), but the default shell in GitHub Actions varies by runner OS. On Windows runners, this will fail. Either specify 'shell: bash' in the step configuration, or use cross-platform commands. For consistency across all matrix builds (including Windows), add 'shell: bash' to the step.

Copilot · 2025-12-22T23:28:20Z

+          - name: darwin-amd64  # macOS Intel
+            os: macos-latest
+            goos: darwin
+            goarch: amd64
+          - name: darwin-arm64  # macOS Apple Silicon
+            os: macos-latest


Both darwin-amd64 and darwin-arm64 builds are configured to use 'macos-latest' as the runner. However, GitHub Actions currently provides separate runner types: macos-latest (Apple Silicon/ARM64) and macos-13 (Intel/AMD64). Building darwin-amd64 on an ARM64 runner works through cross-compilation, but for optimal build performance and to follow best practices, consider using 'macos-13' for darwin-amd64 builds and 'macos-latest' for darwin-arm64 builds. This is optional since cross-compilation works, but it would be more consistent with how the Windows builds are structured.

rustyeddy added 2 commits November 25, 2025 19:03

Test the matrix builds to see if they work

3c98b21

Removed duplicate steps

efc754d

rustyeddy self-assigned this Nov 25, 2025

rustyeddy added the enhancement New feature or request label Nov 25, 2025

rustyeddy added 6 commits November 25, 2025 19:14

Gardner needs to call the latest otto and device packages

4d61681

Fixed go.sum to include recent tagged devices and otto

667ecdf

use relative paths for otto and devices

dc2c883

Merge branch 'main' into packages

cf36e6c

updated topic paths

ae862aa

Go back to using the released versions

f2fbf79

rustyeddy marked this pull request as ready for review December 22, 2025 23:24

Copilot AI review requested due to automatic review settings December 22, 2025 23:24

rustyeddy merged commit 2def1e3 into main Dec 22, 2025
8 of 9 checks passed

rustyeddy deleted the packages branch December 22, 2025 23:24

Copilot started reviewing on behalf of rustyeddy December 22, 2025 23:24 View session

Copilot AI reviewed Dec 22, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Packages#3

Packages#3
rustyeddy merged 8 commits into
mainfrom
packages

rustyeddy commented Nov 25, 2025 •

edited

Loading

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Dec 22, 2025

Uh oh!

Copilot AI Dec 22, 2025

Uh oh!

Copilot AI Dec 22, 2025

Uh oh!

Copilot AI Dec 22, 2025

Uh oh!

Copilot AI Dec 22, 2025

Uh oh!

Copilot AI Dec 22, 2025

Uh oh!

Copilot AI Dec 22, 2025

Uh oh!

Copilot AI Dec 22, 2025

Uh oh!

Copilot AI Dec 22, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

	go build -trimpath -ldflags "-s -w" -o "$OUT" ./...
	go build -trimpath -ldflags "-s -w" -o "$OUT" .

Conversation

rustyeddy commented Nov 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Dec 22, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 22, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 22, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 22, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 22, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 22, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 22, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 22, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 22, 2025

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

rustyeddy commented Nov 25, 2025 •

edited

Loading