screen-haven: add status display for the 3.5in USB panel

[mndavew3]

Screen Haven is a status display for the 480x320 "Turing"-style 3.5-inch USB panel: live bandwidth, system health, network detail, connected devices and a clock, driven by an attached USB mouse. It reads only stock system state (/proc, /sys, the default route, DHCP leases, uci wireless) — no external data, nothing sent anywhere. Source is fetched from a tagged release tarball.

Maintainer: @mndavew3

---

🧪 Run Testing Details

• OpenWrt Version: 25.12.5 (SDK openwrt-sdk-25.12.5-x86-64_gcc-14.3.0_musl.Linux-x86_64)
• OpenWrt Target/Subtarget: x86/64
• OpenWrt Device: none — SDK build only

Built against the 1.1 tarball now referenced by the Makefile (PKG_HASH verified against the release asset): compiles cleanly and produces screen-haven-1.1-r1.apk. The resulting executable was run under the SDK's musl loader and answers the version query the package CI uses:

$ screen-haven --version
screen-haven 1.1
$ screen-haven -V
screen-haven 1.1

so no test-version.sh override is needed.

Not run against the physical panel from this tarball — the display path is untested here.

With -w replaced by -Wall, the package's own sources now emit 6 warnings, all reviewed and benign: four -Wmisleading-indentation in hvn_tri() (two statements sharing a line; the code is correct) and two -Woverflow in hvn_screenshot(), where HVN_W/HVN_H are truncated on purpose to write the little-endian width/height bytes of a bitmap header. They are left visible rather than silenced.

---

✅ Formalities

• I have reviewed the CONTRIBUTING.md file for detailed contributing guidelines.

If your PR contains a patch:

• It can be applied using git am
• It has been refreshed to avoid offsets, fuzzes, etc., using

  make package/<your-package>/refresh V=s

• It is structured in a way that it is potentially upstreamable
  _{(e.g., subject line, commit description, etc.)
  We must try to upstream patches to reduce maintenance burden.}

[openwrt-ai]

Reviewed 1 new commit (1b7dd44). The commit message matches the diff, and all three FormalityCheck runs are green on the head commit.

Four inline notes, none of which look like a hard blocker: two behavioural (hotplug never stops the service on remove, which leaves procd respawning against a vanished node; and the init script hardcodes ttyACM0 while hotplug matches ttyACM*), one build-flag issue (-O2 overrides the configured CONFIG_TARGET_OPTIMIZATION and -w silences all diagnostics on new C code), and one packaging note about the CI generic version check.

The /etc/hotplug.d/tty dispatch itself is correct — procd routes usb-serial events with ^ttyACM there, per hotplug.json — and return 0 in a sourced hotplug script works as intended given hotplug-call sources each script in a subshell. I could not verify the release tarball at the PKG_SOURCE_URL (the source repo is outside this session's access), so PKG_HASH and the asset name are unchecked.

---

Generated by Claude Code

[openwrt-ai]

-O2 overrides the optimization level the user configured: TARGET_CFLAGS:=$(TARGET_OPTIMIZATION)... in rules.mk:202 already puts CONFIG_TARGET_OPTIMIZATION (-Os on most targets) at the front of $(TARGET_CFLAGS), and gcc takes the last -O on the command line. On space-constrained targets this silently builds bigger than the config asks for.

-w then disables every diagnostic, so nothing this new C code does — implicit declarations, format-string mismatches, sign issues in the /proc parsing — will show up in a build log. In-tree packages that compile by hand use -Wall instead (utils/io, utils/i2csfp, utils/sexpect); none of them add their own -O.

Suggested change

	$(TARGET_CC) $(TARGET_CFLAGS) $(TARGET_LDFLAGS) -O2 -w \
	$(TARGET_CC) $(TARGET_CFLAGS) $(TARGET_CPPFLAGS) $(TARGET_LDFLAGS) -Wall \

---

Generated by Claude Code

[mndavew3]

Fixed. Build/Compile now reads:

	$(TARGET_CC) $(TARGET_CFLAGS) $(TARGET_CPPFLAGS) $(TARGET_LDFLAGS) -Wall \

Dropping -O2 leaves CONFIG_TARGET_OPTIMIZATION as the last -O on the line, and TARGET_CPPFLAGS is added so the toolchain include paths come along. -Wall was not free: the package's own sources emit 6 warnings that -w had been hiding — four -Wmisleading-indentation in hvn_tri() (two statements sharing a line, code correct) and two -Woverflow in hvn_screenshot(), where HVN_W/HVN_H are truncated deliberately to write the little-endian width/height bytes of a bitmap header. Both reviewed, both benign, and now visible in the build log instead of suppressed.

[openwrt-ai]

The panel node is hardcoded to ttyACM0, but the hotplug handler triggers on ttyACM* — so the two disagree whenever the panel does not enumerate first. If any other CDC-ACM device is present (a modem, a serial-over-USB MCU, another panel), the panel can land on /dev/ttyACM1; hotplug then fires restart, start_service finds /dev/ttyACM0 missing, return 0 runs, and the service comes up empty with nothing logged. The reverse case is worse: an unrelated modem on ttyACM0 gets handed to screen-haven as if it were the display.

Consider making the node configurable (UCI or /etc/config) or probing for a panel rather than assuming index 0 — and if it stays fixed, the hotplug case should match only the node the init script actually uses so the two cannot drift apart.

Also worth logging when the guard bails, otherwise a non-starting service looks identical to a started one.

---

Generated by Claude Code

[mndavew3]

Fixed — you called it exactly, including the failure mode. start_service no longer names a node; it takes the first ttyACM* that is actually present, so it can no longer disagree with the hotplug rule, and it logs when there is none:

	set -- /dev/ttyACM*
	if [ ! -c "$1" ]; then
		logger -t screen-haven "no /dev/ttyACM* panel present - not starting"
		return 0
	fi
	procd_open_instance
	procd_set_param command /usr/bin/screen-haven "$1"

For what it is worth, this is not hypothetical: on our own hardware a hub swap moved the panel from ttyACM0 to ttyACM1 and a hard-coded path waited forever, which is where this shape comes from.

The one case this does not cover is the one you raised second — an unrelated modem sitting on ttyACM0 still gets picked. Distinguishing them needs identification by USB VID/PID inside the binary rather than a guess in the init script; that belongs in the program and is queued for the next release rather than papered over here.

[openwrt-ai]

Only add is handled, so unplugging the panel leaves the service running. procd_set_param respawn in the init script means procd restarts screen-haven every time it dies on the now-vanished /dev/ttyACM0, and the [ -e "$PANEL" ] guard is in start_service only — it is not re-evaluated on respawn. The result is a respawn loop (five restarts by default before procd gives up, with stdout/stderr piped to syslog each time) instead of a clean stop.

Handling remove also lets the service come back cleanly on replug, since add currently only does anything if the instance is still registered.

Suggested change

	[ "$ACTION" = add ] || return 0
	case "$DEVNAME" in
	ttyACM*)
	/etc/init.d/screen-haven enabled && /etc/init.d/screen-haven restart
	;;
	esac
	case "$DEVNAME" in
	ttyACM*) ;;
	*) return 0 ;;
	esac
	/etc/init.d/screen-haven enabled || return 0
	case "$ACTION" in
	add) /etc/init.d/screen-haven restart ;;
	remove) /etc/init.d/screen-haven stop ;;
	esac

---

Generated by Claude Code

[mndavew3]

Fixed, essentially as suggested:

case "$DEVNAME" in
	ttyACM*) ;;
	*) return 0 ;;
esac

/etc/init.d/screen-haven enabled || return 0

case "$ACTION" in
	add) /etc/init.d/screen-haven restart ;;
	remove) /etc/init.d/screen-haven stop ;;
esac

The respawn-loop reasoning is right — respawn restarts on every exit and the device guard lives in start_service only, so it is never re-evaluated on the way back up.

[openwrt-ai]

The package CI runs a generic runtime check on every installed executable, invoking it with --version/--help and expecting PKG_VERSION (1.0) in the output. screen-haven takes a panel device path as its argument and, from the description, drives a serial display — it is unlikely to print 1.0 for those flags, and if it instead tries to open the argument as a device the check will hang or fail.

Unless the binary already handles --version explicitly, add a utils/screen-haven/test-version.sh that returns 0 for this package to override the generic check. (Note the feed convention of not using grep -q in those scripts, so the matched version line stays visible in the CI log.)

---

Generated by Claude Code

[mndavew3]

Already handled in the source, and now verified rather than assumed. 1.1 answers the version query before it touches hardware (--version, -version, version, -v, -V, and the --help forms). Built with the 25.12.5 x86-64 SDK and run under the SDK's musl loader:

$ screen-haven --version
screen-haven 1.1
$ screen-haven -V
screen-haven 1.1

PKG_VERSION is 1.1 and the string printed is 1.1, so the generic check matches and no test-version.sh override is needed. If a maintainer would still rather see one for explicitness, say so and I will add it.

[openwrt-ai]

Reviewed 1 new commit (45cf0f9, force-pushed over the previously reviewed 1b7dd44). The only change since the last review is the bump 1.0 → 1.1 with a new PKG_HASH, plus DEPENDS:=+zlib and -lz on the link line.

Those three go together correctly: zlib is the package that ships libz (BuildPackage,zlib), a DEPENDS on a library also establishes the build ordering so no PKG_BUILD_DEPENDS is needed, and -lz sits after the objects. No extra include/library flags are required for it either — OpenWrt patches the cross-gcc specs with -idirafter %:getenv(STAGING_DIR /usr/include) and -L %:getenv(STAGING_DIR /usr/lib) via patch-specs.sh:38-41, so zlib.h and libz.so resolve from the staging dir. PKG_RELEASE staying at 1 is what a version bump calls for. The hardcoded https://github.com/.../releases/download/... URL is also fine as-is: @GITHUB expands to raw.githubusercontent.com, which cannot serve release assets, and 161 in-tree Makefiles use the plain form for release downloads.

All three FormalityCheck runs are green on the head commit.

Commit checks

• 45cf0f9 "screen-haven: add status display for the 3.5in USB panel" — the second half of the body ("Updated to 1.1, which fixes the two problems the package CI found: …") is a changelog against a 1.0 that never lands in the tree. This commit adds the package, and 1.1 is the first version git ever sees, so git log will show a brand-new package apologising for CI failures of a version that does not exist here. The technical content is worth keeping (why zlib is a dependency, why the binary answers --version) — it just reads better stated as properties of the package than as a delta from 1.0. Not a blocker.

One inline nit on the stale version reference in the PR description. Nothing in this round blocks a merge.

Status of the earlier threads, for tracking: the -O2 -w build-flag note, the hardcoded ttyACM0 vs. ttyACM* mismatch, and the missing hotplug remove handling are all untouched by the force-push and still apply. The fourth (generic version check / test-version.sh) appears addressed by the binary now answering --version — assuming its output contains the literal 1.1, which I cannot check from here.

PKG_HASH remains unverified: the source repository is outside this session's access, so the tarball, its checksum and the release asset name are unchecked.

---

Generated by Claude Code

[openwrt-ai]

nit: the PR description still describes the 1.0 build — "produces screen-haven-1.0-r1.apk" — while the Makefile now packages 1.1 with a different PKG_HASH, so the stated test evidence no longer matches what a maintainer would build from this branch. Worth re-running the SDK build against 1.1 and updating the description (or saying plainly that the 1.1 tarball has not been build-tested yet).

While editing it: the description also still carries the unfilled template block below the summary — **Maintainer:** @ with no handle, an empty Description, and empty OpenWrt Version / Target / Device fields — which reads as a second, contradictory set of answers to the ones given at the top.

---

Generated by Claude Code

[mndavew3]

Both fixed. The description now states the 1.1 build (SDK openwrt-sdk-25.12.5-x86-64_gcc-14.3.0_musl.Linux-x86_64, x86/64, PKG_HASH verified against the release asset, produces screen-haven-1.1-r1.apk), records that the executable was actually run for the version check, and says plainly that it has not been run against the physical panel from this tarball. The duplicated, unfilled template block is gone and the Run Testing fields are filled in.

[openwrt-ai]

Reviewed 1 new commit (9cb11f0, a squash/force-push over the previously reviewed 45cf0f9); no new issues found.

The diff since the last review is exactly the three fixes that were requested, and each is implemented correctly:

• Build flags — -O2 -w is replaced by -Wall, with $(TARGET_CPPFLAGS) added and -lz still trailing the objects. CONFIG_TARGET_OPTIMIZATION is now the last -O on the line as intended.
• screen-haven.init — no fixed device node; set -- /dev/ttyACM* with an [ ! -c "$1" ] guard (correctly catches the non-matching literal-glob case) and a logger line when no panel is present.
• screen-haven.hotplug — remove now stops the service, so an unplug no longer leaves procd respawning against a vanished node.

All three FormalityCheck runs are green on the head commit. As before, PKG_HASH and the release asset remain unverified (the source repo is outside this session's access). The remaining wrong-device case (an unrelated CDC-ACM device on the first ttyACM*) is acknowledged in the thread as deferred to VID/PID identification in the binary.

---

Generated by Claude Code