meta-opencentauri: flesh out helixscreen recipe + add libhv support#148
meta-opencentauri: flesh out helixscreen recipe + add libhv support#148prestonbrown wants to merge 4 commits intoOpenCentauri:mainfrom
Conversation
Replaces the 9-line helixscreen stub with a full recipe that builds, links
against system libraries (libhv, openssl, spdlog, fmt, alsa-lib, libusb1,
wpa-supplicant, libnl), and installs a runnable .ipk with SysV init hook.
Recipe files:
- recipes-support/libhv/libhv_1.3.4.bb
cmake-inherit recipe for ithewei/libhv tagged release. Applies a
dns-resolver fallback patch helixscreen requires (provides
dns_resolv_resolve symbol for statically-linked edge cases).
SOVERSION fixup in do_install:append renames the unversioned
libhv.so so it lands in the runtime package with proper symlinks.
- recipes-connectivity/wpa-supplicant/wpa-supplicant_%.bbappend
Clears DISABLE_STATIC so upstream wpa-supplicant_2.10.bb actually
builds libwpa_client.a. Without this, any screen UI that links
wpa_ctrl (helixscreen, guppyscreen) fails at link time with
'cannot find -lwpa_client'. Only changes one variable, no other
recipe behavior touched.
- recipes-apps/helixscreen/helixscreen_0.99.bb (replaces _0.1.bb)
DEPENDS on system libs + EXTRA_OEMAKE wires PLATFORM_TARGET=yocto
into helixscreen's Makefile. do_install places the binary +
supervisor helpers + runtime assets under /usr/share/helixscreen/,
with /usr/bin/helix-screen symlink and /etc/init.d/helixscreen
hooked via update-rc.d (S80/S20). Matches the sibling-recipe
convention used by guppyscreen/grumpyscreen.
- recipes-apps/helixscreen/files/helixscreen.init
SysV init script from helixscreen's tree with HELIX_CONFIG_DIR
export added up-front so user settings persist into klipper's
~/printer_data/config/helixscreen regardless of the RO squashfs
install tree. DAEMON_DIR is sed-rewritten to the packaged path
at install time.
Tested: bitbake helixscreen produces a clean .ipk (~93 MB) for
cortexa7t2hf-neon-vfpv4 with GNU_HASH present and split-debug extraction
working. Recipe CFLAGS/LDFLAGS flow through from bitbake into the
Makefile's YOCTO_BUILD path, which requires helixscreen main at
ea728a294 or newer.
Refs OpenCentauri#145.
1397574 to
bb77853
Compare
jamesturton
left a comment
There was a problem hiding this comment.
It feels like the init script has not been tested as I saw a lot of issues with it. Other than that there are a few minor nit picks with the recipes files, but I am excited to see how this compares resource wise to grumpscreen!
|
|
||
| do_install() { | ||
| install -d ${D}${HELIX_DIR}/bin | ||
| install -m 0755 ${B}/build/yocto/bin/helix-screen ${D}${HELIX_DIR}/bin/helix-screen |
There was a problem hiding this comment.
Why are you installing a binary to ${datadir}? This breaks FHS. Should be installed to ${bindir} instead
| # Optional supervisor binaries — ship if the build produced them (splash | ||
| # + watchdog are targets built by the Makefile on embedded platforms). | ||
| for exe in helix-splash helix-watchdog; do | ||
| [ -x ${B}/build/yocto/bin/$exe ] && install -m 0755 ${B}/build/yocto/bin/$exe ${D}${HELIX_DIR}/bin/ |
There was a problem hiding this comment.
Why are you testing if the script exists? Don't you know that it's there??
| install -d ${D}${HELIX_DIR}/config | ||
| for f in default_layout.json helix_macros.cfg helixscreen.env \ | ||
| printer_database.json printing_tips.json settings.json.template; do | ||
| [ -f ${S}/config/$f ] && install -m 0644 ${S}/config/$f ${D}${HELIX_DIR}/config/ |
There was a problem hiding this comment.
These should be in ${sysconfdir}, preferably in /etc/klipper/config/ so it can be easily configured from web interface
| # Runtime deps — Klipper + Moonraker are the upstream ecosystem; without them | ||
| # helix-screen launches but has nothing to talk to. | ||
| RDEPENDS:${PN} = " \ | ||
| klipper \ |
There was a problem hiding this comment.
Does helixscreen talk directly to klipper or does it use the moonraker api?
| # libhv generates hv/json.hpp etc. into include/hv at configure time; | ||
| # cmake install copies them to ${includedir}/hv. Also ship the CMake | ||
| # package configuration so downstream cmake projects can find_package(hv). | ||
| FILES:${PN} += "${libdir}/cmake/hv" |
There was a problem hiding this comment.
You shouldn't be including header files in the runtime package, they should go in the dev package. If you are unfamiliar with writing yocto recipes for libraries you can look at the yocto docs here: https://docs.yoctoproject.org/scarthgap/dev-manual/libraries.html#working-with-libraries
|
|
||
| NAME="helixscreen" | ||
| DESC="HelixScreen 3D Printer Touch UI" | ||
| DAEMON_DIR="/opt/helixscreen" # Updated by installer for different firmware |
There was a problem hiding this comment.
Why are you using /opt/ here and updating this value somewhere else?? That's not exactly very easy to follow what's going on...
|
|
||
| # Ensure HOME is set (BusyBox init may not set it). | ||
| # Without HOME, config backup falls back to volatile /tmp/ and is lost on reboot. | ||
| : "${HOME:=/root}" |
There was a problem hiding this comment.
Doesn't exist in cosmos, maybe you meant /home/root?
| # Point user-writable settings at klipper's printer_data so they persist even | ||
| # when the baseline install lives on a read-only rootfs (e.g., the cosmos | ||
| # squashfs). Honored by helix::Config::init in src/system/config.cpp. | ||
| : "${HELIX_CONFIG_DIR:=${HOME}/printer_data/config/helixscreen}" |
There was a problem hiding this comment.
This doesn't exist either??
|
|
||
| # Ensure config backup directory exists (survives Moonraker's shutil.rmtree wipe). | ||
| # On systemd systems StateDirectory= creates /var/lib/helixscreen/; on SysV we do it here. | ||
| mkdir -p /var/lib/helixscreen 2>/dev/null || mkdir -p "${HOME}/.helixscreen" 2>/dev/null || true |
There was a problem hiding this comment.
Why are you trying to write into 2 dirs? It feels like this init script hasn't been tested at all on cosmos...
Addresses all review feedback from OpenCentauri#148: helixscreen_0.99.bb: - Binary installed to ${bindir} (FHS), not ${datadir}/bin + symlink - Init script overhauled: guppy-style start-stop-daemon, HELIX_CONFIG_DIR and HELIX_DATA_DIR set via sed placeholders, no /var/lib mkdir (volatile on cosmos), no '[ -x $DAEMON ] || exit 0' check on a file we just installed - RO data (ui_xml, assets including seed configs) at ${datadir}/helixscreen - User-writable config at ${sysconfdir}/klipper/config/helixscreen (CONFFILES — preserved across opkg upgrade), seeded with cc1.json preset - Dev-only test gcodes excluded from .ipk (tar --exclude, saves ~50 MB) - Explicit do_compile() for externalsrc compatibility - RDEPENDS: dropped klipper (transitive via moonraker), kept moonraker + wpa-supplicant - INITSCRIPT_PARAMS = disable (gui-switcher picks the active UI) - pkg_postinst: migrates writable state from /user-resource/helixscreen tarball install (settings.json, telemetry_*.json, tool_spools, themes, custom_images) into the new ${HELIX_USER_CONFIG_DIR}; seeds are NOT copied (find_readable falls back to ${HELIX_DATA_DIR}/assets/config/) helixscreen.init: - Replaced 275-line multi-platform init with 80-line cosmos-specific script using start-stop-daemon. Path placeholders (@HELIX_BINDIR@, @HELIX_DATA_DIR@, @HELIX_USER_CONFIG_DIR@) get sed'd by do_install. libhv_1.3.4.bb: - New patch: libhv-set-soversion.patch adds VERSION/SOVERSION to the cmake shared library target so DT_SONAME=libhv.so.1 is stamped into the ELF. Without this, helix-screen gets DT_NEEDED=libhv.so (unversioned, dev-only) and fails to load at runtime. - Dropped patchelf-native dep + the manual do_install rename/symlink dance — cmake handles the chain natively now. - FILES:${PN}-dev += cmake/hv (was incorrectly in runtime package). Verified on CC1 (192.168.1.52) via manual .ipk extract + env-var launch: app starts, connects to Moonraker, loads theme/tips/printer DB from ${HELIX_DATA_DIR}/assets/config/, writes state to ${HELIX_CONFIG_DIR}, postinst migration copies tarball user state correctly.
Code review fixes:
helixscreen.init:
- Move platform_pre_start/platform_post_stop fallback no-op definitions
BEFORE the source so hooks-cc1.sh can override them. Previously the
unconditional redefinition after the source wiped whatever the hooks
file defined.
- 'return 1' → 'exit 1' in the status case — return outside a function
is a syntax error in dash/ash (BusyBox).
- Remove now-redundant 'type ... >/dev/null 2>&1 &&' guards since the
fallback functions are always defined.
helixscreen_0.99.bb pkg_postinst:
- settings.json from the legacy tarball install now OVERWRITES the cc1
preset that do_install placed. The user's tarball settings contain their
actual preferences and calibration; the shipped cc1.json default is only
appropriate for fresh installs with no prior state.
Updated — addresses all review feedbackThanks for the thorough review, @jamesturton. You were right that the init script wasn't tested on cosmos — I've now verified everything on an actual CC1 device. What changedhelixscreen_0.99.bb (complete rewrite):
helixscreen.init (rewritten from scratch — 80 lines, not 275):
libhv_1.3.4.bb:
Background: RO/RW config splitThe helixscreen repo now separates shipped read-only seed files ( Testing
Still needed (separate from this PR)
|
|
I was working on another code review but I happened to read the readme for helixscreen that it uses around 15mb ram. This is a huge jump compared to the 3mb which grumpscreen, our current default ui uses today. While we have zram enabled in the current builds increasing the memory pressure so greatly will dramatically increase the risk of TTC errors, especially with the upcoming mmu support. I'll have a talk with the other crew and see if this is a direction we want to take, or if we continue to focus on the reliability of the core features. Thanks for taking an interest in the project! |
Summary
helixscreen_0.1.bbstub from config-manager: allow helixscreen as screen_ui value #145 with a working recipe that produces a deployable.ipklibhv_1.3.4.bb(cmake inherit, ithewei/libhv, OpenSSL + HTTP client/server enabled, SOVERSION fixup); helixscreen links against itwpa-supplicant_%.bbappendthat clearsDISABLE_STATICso `libwpa_client.a` actually builds (benefits guppyscreen too, which already links `-lwpa_client` by building it in-tree)Install layout
Binary + assets under `/usr/share/helixscreen/` with `/usr/bin/helix-screen` symlink and `/etc/init.d/helixscreen` (SysV, S80/S20 via update-rc.d) — matches the convention used by the existing `guppyscreen` and `grumpyscreen` recipes in this layer.
Settings persistence on the RO rootfs
The init script exports `HELIX_CONFIG_DIR=$HOME/printer_data/config/helixscreen` before starting helix-screen. helixscreen's `Config::init` honors this var and routes writes to the klipper printer_data directory, so user settings persist even though `/usr/share/helixscreen/` lives on the read-only squashfs.
Testing
`bitbake helixscreen` on a scarthgap build for CC1 produces a clean ~93 MB .ipk; all tasks succeed, `do_package_qa` passes (GNU_HASH + split-debug both clean).
Dependency on helixscreen SRCREV
The pinned `SRCREV` (`ea728a294`) is on prestonbrown/helixscreen `main`. It carries the `PLATFORM_TARGET=yocto` Makefile path, LDFLAGS-preservation fixes for splash/watchdog, and the `HELIX_CONFIG_DIR` config-redirect.
Refs #145.
Test plan