Skip to content

fix(install): pass /dev/cec0 through on pi2/pi3/pi3-64/pi4-64 - #3273

Open
vpetersson-bot wants to merge 1 commit into
Screenly:masterfrom
vpetersson-bot:split/cec-passthrough-pi
Open

fix(install): pass /dev/cec0 through on pi2/pi3/pi3-64/pi4-64#3273
vpetersson-bot wants to merge 1 commit into
Screenly:masterfrom
vpetersson-bot:split/cec-passthrough-pi

Conversation

@vpetersson-bot

Copy link
Copy Markdown
Contributor

Issues Fixed

Refs #3267 — this is the root cause half. The user-visible status-string half is in a separate PR.

⚠️ Read this first

This changes device passthrough during real OTA upgrades, and I have NOT run an actual upgrade. It is deliberately isolated from the other fixes in this batch so it can be reviewed, merged and reverted on its own risk profile. Please treat it as needing a real upgrade run on a Pi before merge — see "What I could not verify" below.

Description

bin/upgrade_containers.sh already rewrites the compose devices: entry from /dev/vchiq to /dev/cec0 — but its case "$DEVICE_TYPE" only matches pi5 and x86|arm64. pi2, pi3, pi3-64 and pi4-64 fall through the case entirely and keep the vchiq node, which libcec cannot open on a vc4-kms kernel.

That is why display power has never worked on those boards, and why operators saw a bare 'CEC error'. It was a targeted Pi 5 fix (#2863) that was never extended, and its comment — "Pi 1-4 reaches libcec via /dev/vchiq (closed-firmware VideoCore IV)" — is stale: these boards now boot dtoverlay=vc4-kms-v3d.

Evidence

Measured on the pi4-64 testbed. The host has a real /dev/cec0 (vc4_hdmi, fef00700.hdmi) plus /dev/cec1; the server and celery containers see only /dev/vchiq.

With /dev/cec0 passed into a throwaway --rm container:

Stage Result Time
cec.init() OK 1187ms
cec.Device(TV) OK 1430ms
tv.is_on() raises OSError: Power status not found 286ms
total 2910ms

cec.list_adapters()['Linux']. Control, same image, vchiq only → fails in 82ms. On the pi3-32, vchiq does not even fail — libcec hangs past the caller's timeout.

So the adapter genuinely works when passed through. The is_on() raise is expected: no monitor in the testbed fleet supports CEC, and that is precisely the case the companion PR reports as 'No CEC display detected' rather than an error.

Safety of the change

  • Guarded on [ -e /dev/cec0 ], and leaves vchiq untouched otherwise. Compose refuses to start a container whose listed host node is missing, so an unconditional rewrite would be a fleet-wide outage on any board without it. This mirrors the existing x86|arm64 branch's pattern, which has been in production.
  • /dev/cec1 deliberately not added. It exists on some of these boards and not others, and the second HDMI output is not needed to read display power. (The pi5 branch adds both, which is safe there because a Pi 5 always exposes both.)
  • Worst case if /dev/cec0 is absent: no change at all from today's behaviour.

A tradeoff worth knowing before merging

This makes the CEC query ~36× slower where it starts working: ~2950ms versus the 82ms fast-fail. Still ~10× inside the task's 30s soft limit and inside the 10s subprocess bound, so it is safe — but it is no longer negligible, and it puts real work into a beat that runs every 5 minutes on low-RAM boards.

If that trade is unwelcome, the alternative is the opposite direction: make cec_available() require /dev/cec0 so these boards report 'Not available' and skip the probe entirely. That gives a correct status and less work, at the cost of CEC never working on them. I went with making the feature work, but it is genuinely a product call rather than a technical one.

What I could not verify

  • The upgrade path itself was never run. I verified the sed against a sample compose file and reasoned from the existing x86|arm64 precedent, which uses the identical guard. A real MODE=pull bin/upgrade_containers.sh on a Pi 2 and a Pi 4 is the missing step.
  • No CEC-capable display exists in the testbed fleet, so a successful power reading is unobservable anywhere here. What is proven is that the adapter opens and responds; what is unproven is an end-to-end on/off against a real TV.
  • The generic arm64 board (Rock Pi 4) was off-network throughout, and it is the only board on that image — though arm64 already matched the pre-existing branch, so it is unaffected by this change.

Checklist

  • I have performed a self-review of my own code.
  • New and existing unit tests pass locally and on CI with my changes.
  • I have done an end-to-end test for Raspberry Pi devices.
  • I have tested my changes for x86 devices.
  • I added a documentation for the changes I have made (when necessary).

Unit tests: there are none for this shell path, and I did not add a harness for one case arm. Device boxes unticked deliberately — the adapter was validated on a Pi 4, but the upgrade script that performs the rewrite was not executed on any board. x86 is untouched by this change (it already had its branch).

The vchiq -> cec0 rewrite only matched pi5 and x86|arm64, so every other
Pi board fell through the case and kept the /dev/vchiq node libcec cannot
open on a vc4-kms kernel. That is the root cause of display power never
working on them, and of the bare 'CEC error' operators saw. It was a
targeted Pi 5 fix (Screenly#2863) that was never extended.

Measured on the pi4-64 testbed with /dev/cec0 passed into a throwaway
container: libcec opens the adapter (init 1.19s, Device 1.43s) and the
query completes in ~2.9s. With vchiq only it fails in 82ms, or hangs
until the caller's timeout on the pi3-32.

Guarded on [ -e /dev/cec0 ], leaving vchiq untouched otherwise, because
compose refuses to start a container whose listed host node is missing —
an unconditional rewrite would be a fleet-wide outage. /dev/cec1
deliberately not added: present on some of these boards and not others,
and not needed to read display power.

Refs Screenly#3267.
@vpetersson-bot
vpetersson-bot requested a review from a team as a code owner August 7, 2026 08:23
@vpetersson-bot vpetersson-bot self-assigned this Aug 7, 2026
@sonarqubecloud

sonarqubecloud Bot commented Aug 7, 2026

Copy link
Copy Markdown

@codecov

codecov Bot commented Aug 7, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
⚠️ Please upload report for BASE (master@82c5def). Learn more about missing BASE report.

Additional details and impacted files
@@            Coverage Diff            @@
##             master    #3273   +/-   ##
=========================================
  Coverage          ?   90.70%           
=========================================
  Files             ?       76           
  Lines             ?     8439           
  Branches          ?      896           
=========================================
  Hits              ?     7655           
  Misses            ?      563           
  Partials          ?      221           

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@vpetersson
vpetersson requested a review from Copilot August 7, 2026 08:35

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot wasn't able to review any files in this pull request.


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

@vpetersson
vpetersson requested a review from Copilot August 7, 2026 09:35

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot wasn't able to review any files in this pull request.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants