fix(cec): support HDMI-CEC display power on Pi 4/5 second micro-HDMI port#3085
fix(cec): support HDMI-CEC display power on Pi 4/5 second micro-HDMI port#3085federicopratto wants to merge 3 commits into
Conversation
|
Thank you for your contribution, but could you please translate this to English? We can't accept contributions with Spanish comments in the code base or the PR. |
vpetersson
left a comment
There was a problem hiding this comment.
Thanks for digging into this — the diagnostics change and the new tests are good. But there's a hard blocker in bin/upgrade_containers.sh plus a couple of things worth confirming before this can land. Details inline.
Summary
- 🔴 The
x86|arm64branch body is a literal...placeholder — it aborts the upgrade (#!/bin/bash -e,...→ command-not-found → exit 127) on every x86/arm64 device, and deletes the logic that strips the/dev/vchiqmount so those boards can boot at all. Must be restored. - 🟡 The primary remap path depends on
cec-ctlbeing installed on the host, which Anthias doesn't manage — on a stock Pi OS host it silently falls back to the old broken behavior. - 🟡 The fallback mounts both
/dev/cec0and/dev/cec1underdevices:, which fails container start if a host exposes only one — andpi4-64is unverified on real hardware. - 🟢 Minor:
cec-ctl --playbackis a mutating probe; Spanish comments in an otherwise-English file.
| fi | ||
| ;; | ||
| x86|arm64) | ||
| ... |
There was a problem hiding this comment.
🔴 Blocker — this is a literal ... placeholder, not real code.
The shebang is #!/bin/bash -e, so on any x86/arm64 device ... resolves to command not found (exit 127) and the entire upgrade aborts. bash -n passes (it's a syntactically valid command name), so CI stays green while every x86/arm64 upgrade breaks at runtime.
It also deletes the real x86/arm64 logic that was on master:
x86|arm64)
if [ -e /dev/cec0 ]; then
sed -i 's|/dev/vchiq:/dev/vchiq|/dev/cec0:/dev/cec0|g' ...
else
sed -i '/devices:/ {N; /\n.*\/dev\/vchiq:\/dev\/vchiq/d}' ...
fiThat else branch is essential: x86/arm64 hosts have no /dev/vchiq, and since the template lists it under devices:, leaving the mount in place makes the container fail to start. Please restore the original branch (it looks like an accidental edit leftover).
There was a problem hiding this comment.
You're right — that was an editing slip on our end, not intentional.
Restored the original x86|arm64 branch verbatim. Pushed.
| sed -i 's|/dev/vchiq:/dev/vchiq|/dev/cec0:/dev/cec0|g' \ | ||
| pi4-64|pi5) | ||
| CEC_DEV="" | ||
| if command -v cec-ctl >/dev/null 2>&1; then |
There was a problem hiding this comment.
🟡 This remap path only triggers when cec-ctl exists on the host. cec-ctl ships in v4l-utils, which isn't installed by default on Raspberry Pi OS, and Anthias is userspace-only (doesn't manage host packages). On a stock Pi host this takes the else fallback — which, per your own comment below, won't work if the live port is /dev/cec1.
So for the majority of real devices this likely degrades to the old broken behavior. Can you confirm whether the target hosts actually ship cec-ctl? If not, the fix's real-world reach is much narrower than the PR implies and should be documented (or the detection moved somewhere that has cec-ctl available).
There was a problem hiding this comment.
Confirmed, and you're right to push on this. Checked our own ansible
playbooks — grep -rn "v4l-utils\|cec-ctl\|cec-utils" ansible/
returns nothing, so Anthias's installer never provisions it. On my
test Pi 5, apt-mark showmanual lists v4l-utils as manually
installed, not part of the base image — I'd installed it myself while
diagnosing this, so a genuinely stock host almost certainly doesn't
have it.
So yes: on a typical install today, this falls through to the
degraded fallback in most cases, same as you suspected. Two ways I
see to actually close that gap rather than just document it:
- Have
upgrade_containers.shinstallv4l-utilsitself on
pi4-64|pi5whencec-ctlis missing — but that crosses the
"Anthias doesn't manage host packages" line you mentioned, so I
don't want to do that without a green light. - Run the
cec-ctlprobe from inside a throwaway container using
the server image instead (addingv4l-utilsto
Dockerfile.server/Dockerfile.test, passing
--device=/dev/cec0 --device=/dev/cec1to adocker run), so
detection never depends on anything installed on the host at all.
(2) feels more in line with the project's existing philosophy, but
it's more surface area than this PR currently touches. Happy to do
whichever you'd rather see — or land this as-is with the limitation
called out explicitly (comment + PR description) and open a
follow-up issue for the container-based probe.
| # resulta ser /dev/cec1, va a seguir sin funcionar hasta | ||
| # el próximo upgrade que sí pueda detectarlo; es una | ||
| # limitación conocida de este fallback degradado. | ||
| sed -i 's|^\([[:space:]]*\)- "/dev/vchiq:/dev/vchiq"$|\1- "/dev/cec0:/dev/cec0"\n\1- "/dev/cec1:/dev/cec1"|' \ |
There was a problem hiding this comment.
🟡 This fallback mounts both /dev/cec0 and /dev/cec1. Because these are devices: entries (not volume binds), container start fails if a host exposes only one node. This was safe for pi5 (reliably exposes both), but you're extending it to pi4-64, which is unverified on real hardware per your testing notes. Please confirm a real Pi 4 always exposes both nodes, or guard each mount on [ -e ].
There was a problem hiding this comment.
Good catch. Restructured the fallback to mount only whichever
/dev/cecN nodes actually exist on the host (guarded with [ -e ]),
instead of assuming both are always present. Tested all four
combinations (both / cec0-only / cec1-only / neither) against a
disposable compose fixture to confirm the resulting devices: block
is correct in each case, and validated end-to-end on the test Pi 5.
Pushed.
| if command -v cec-ctl >/dev/null 2>&1; then | ||
| for DEV in /dev/cec0 /dev/cec1; do | ||
| [ -e "$DEV" ] || continue | ||
| PHYS_ADDR=$(cec-ctl -d "$DEV" --playback --logical-address 2>/dev/null \ |
There was a problem hiding this comment.
🟢 Minor: cec-ctl --playback is a mutating probe — it reconfigures the adapter as a Playback device and claims a logical address just to read the physical address. A non-mutating query (e.g. --show-topology, or reading status without --playback) would avoid changing adapter state during the upgrade. Low risk, but worth noting.
There was a problem hiding this comment.
Tested both --show-topology and a bare invocation (no action flag at
all). --show-topology does an active bus scan (Give System
Information exchange with the TV, builds a topology tree) — clearly
more than we need just to read a physical address. The bare
cec-ctl -d /dev/cecX only prints the Driver Info block (Physical
Address included) without sending anything over the CEC bus, so
that's what the script uses now.
One nuance worth flagging: on my test Pi, the adapter already shows
an existing Logical Address claim ("Recording Device 1") even with
the bare command — that's leftover state from earlier manual cec-ctl
testing during this investigation, not something either invocation
causes by itself. The bare read doesn't add to it or trigger any new
bus communication, which is the part that actually matters here.
Pushed.
| fi | ||
|
|
||
| if [ -n "$CEC_DEV" ]; then | ||
| # libcec solo prueba /dev/cec0 — no enumera /dev/cec1 |
There was a problem hiding this comment.
🟢 Minor: these new inline comments are in Spanish while the rest of the file is English. Suggest translating for consistency.
There was a problem hiding this comment.
Good catch — translated all the new inline comments to English in
the same commit that addressed the other points above. Pushed.
…p mutating CEC probe - Restore the x86|arm64 case body that was accidentally replaced with a placeholder, breaking the upgrade on every non-Pi device. - Guard each /dev/cecN mount in the pi4-64|pi5 fallback path with [ -e ], since devices: entries fail container start if a listed host path doesn't exist and pi4-64 isn't confirmed to always expose both nodes. - Swap cec-ctl --playback for a bare invocation when probing physical address, avoiding the active CEC bus scan / logical-address claim that --playback (and --show-topology) trigger. - Translate inline comments to English for consistency with the rest of the file.
|



Fixes #2863
What this fixes
HDMI-CEC display power (#2886, experimental) still doesn't work on Raspberry Pi 4/5 when the display is connected to the second micro-HDMI port. Symptom: "Display Power (CEC): CEC error" / "Unknown" in System Info, and the Settings buttons do nothing.
Root cause
Pi 4 and Pi 5 each expose two kernel CEC adapters, one per micro-HDMI port (
/dev/cec0,/dev/cec1), but only the port that's actually connected gets a valid physical address — the other one stays atf.f.f.f. The Pythonceclibrary Anthias uses doesn't enumerate both: it specifically looks at/dev/cec0. The fix in #2863 mounts both real paths into the container, but that's not enough — if the TV is on the second port,cec.init()either opens the wrong adapter, or finds nothing at all.Confirmed on hardware (Raspberry Pi 5, kernel 6.18.34-rpt-rpi-2712):
Fix
bin/upgrade_containers.shnow usescec-ctlto detect which of the two/dev/cecNdevices reports a valid physical address, and remaps it to the fixed path/dev/cec0inside the container — regardless of which real index it has on the host. Ifcec-ctlisn't available, it falls back to the previous behavior (mounting both under their real names: degraded, not broken). Thepi5logic is also extended topi4-64, sinceansible/roles/system/templates/config.txt.j2confirms both usevc4-kms-v3d.diagnostics.py::cec_available()now also recognizes/dev/cec1, as a safeguard for the fallback scenario above.Testing
is_on()) and the commands (standby()/power_on()) — the TV physically turns off and on.test_cec_available_true_when_only_cec1_presentintests/test_diagnostics.py— passes (37/37 in the file).test_display_power_section_visible_with_cec1_only(+cec1_stub_devicefixture) intests/test_app.py, following the same pattern as the existing/dev/vchiqtest. I couldn't run it locally due to a Docker infrastructure issue unrelated to this change (Dockerfile rendering for the x86 target); it should run fine in CI — confirmation during review would be appreciated.pi4-64extension is justified by architecture but not verified on real Pi 4 hardware. Confirmation from someone with one on hand is very welcome.Files changed
bin/upgrade_containers.shsrc/anthias_server/lib/diagnostics.pytests/test_diagnostics.pytests/test_app.py