Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
61 changes: 58 additions & 3 deletions bin/upgrade_containers.sh
Original file line number Diff line number Diff line change
Expand Up @@ -163,9 +163,64 @@ cat /home/${USER}/anthias/docker-compose.yml.tmpl \
# pre-fix behaviour of dropping the bind mount entirely). Fixes
# the "CEC error" toast on Pi 5 reported in issue #2863.
case "$DEVICE_TYPE" in
pi5)
sed -i 's|^\([[:space:]]*\)- "/dev/vchiq:/dev/vchiq"$|\1- "/dev/cec0:/dev/cec0"\n\1- "/dev/cec1:/dev/cec1"|' \
/home/${USER}/anthias/docker-compose.yml
pi4-64|pi5)
CEC_DEV=""
if command -v cec-ctl >/dev/null 2>&1; then

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 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).

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

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:

  1. Have upgrade_containers.sh install v4l-utils itself on
    pi4-64|pi5 when cec-ctl is 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.
  2. Run the cec-ctl probe from inside a throwaway container using
    the server image instead (adding v4l-utils to
    Dockerfile.server/Dockerfile.test, passing
    --device=/dev/cec0 --device=/dev/cec1 to a docker 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.

for DEV in /dev/cec0 /dev/cec1; do
[[ -e "$DEV" ]] || continue
PHYS_ADDR=$(cec-ctl -d "$DEV" 2>/dev/null \
| grep "Physical Address" | head -1 | awk -F: '{print $2}' | xargs)
if [[ -n "$PHYS_ADDR" ]] && [[ "$PHYS_ADDR" != "f.f.f.f" ]]; then
CEC_DEV="$DEV"
break
fi
done
fi

if [[ -n "$CEC_DEV" ]]; then
# libcec only ever probes /dev/cec0 — it doesn't enumerate
# /dev/cec1, even when that's the port actually connected
# to the TV (confirmed on hardware: with only /dev/cec1
# mounted under its real name, cec.init() raises "No
# default adapter found"). Remap whichever port is live to
# the fixed path /dev/cec0 inside the container, regardless
# of which physical micro-HDMI port it corresponds to on
# the host.
sed -i "s|^\([[:space:]]*\)- \"/dev/vchiq:/dev/vchiq\"\$|\1- \"$CEC_DEV:/dev/cec0\"|" \
/home/${USER}/anthias/docker-compose.yml
else
# cec-ctl isn't on the host, or no port reported a valid
# physical address (e.g. the TV was off during the
# upgrade). Mount whichever real /dev/cecN nodes actually
# exist on the host, under their real names — devices:
# fails container start if a listed host path doesn't
# exist, so we can't assume both are always present.
# If the live port turns out to be /dev/cec1, this will
# keep not working until a future upgrade run can detect
# it (cec-ctl becomes available, or the TV is on); that's
# a known limitation of this degraded fallback.
MOUNT_REPL=""
if [[ -e /dev/cec0 ]]; then
MOUNT_REPL='\1- "/dev/cec0:/dev/cec0"'
fi
if [[ -e /dev/cec1 ]]; then
if [[ -n "$MOUNT_REPL" ]]; then
MOUNT_REPL="${MOUNT_REPL}\\n\\1- \"/dev/cec1:/dev/cec1\""
else
MOUNT_REPL='\1- "/dev/cec1:/dev/cec1"'
fi
fi

if [[ -n "$MOUNT_REPL" ]]; then
sed -i "s|^\([[:space:]]*\)- \"/dev/vchiq:/dev/vchiq\"\$|${MOUNT_REPL}|" \
/home/${USER}/anthias/docker-compose.yml
else
# Neither node exists — same situation as a board
# with no CEC adapter at all; drop the mount entirely.
sed -i '/devices:/ {N; /\n.*\/dev\/vchiq:\/dev\/vchiq/d}' \
/home/${USER}/anthias/docker-compose.yml
fi
fi
;;
x86|arm64)
if [ -e /dev/cec0 ]; then
Expand Down
6 changes: 5 additions & 1 deletion src/anthias_server/lib/diagnostics.py
Original file line number Diff line number Diff line change
Expand Up @@ -175,7 +175,11 @@ def cec_available() -> bool:
means the adapter *could* work, not that it will: the actual
success/failure is surfaced by ``set_display_power``'s toast.
"""
return os.path.exists('/dev/cec0') or os.path.exists('/dev/vchiq')
return (
os.path.exists('/dev/cec0')
or os.path.exists('/dev/cec1')
or os.path.exists('/dev/vchiq')
)


def get_uptime() -> float:
Expand Down
58 changes: 52 additions & 6 deletions tests/test_app.py
Original file line number Diff line number Diff line change
Expand Up @@ -1865,11 +1865,13 @@ def test_skip_buttons_publish_correct_command(
# 8. Display power (experimental, HDMI-CEC) — issue #2575
# ---------------------------------------------------------------------------
#
# The section is gated on cec_available(), which stats /dev/cec0 and
# /dev/vchiq. Neither exists in the test container by default, so the
# section is hidden on every other settings test. To exercise the
# visible state we stub /dev/vchiq with a plain file before navigating
# and remove it on teardown.
# The section is gated on cec_available(), which stats /dev/cec0,
# /dev/cec1, and /dev/vchiq. The /dev/cec1 check covers Pi 4/5 hosts
# where the display is wired to the second micro-HDMI port (issue
# #2863). None of these exist in the test container by default, so
# the section is hidden on every other settings test. To exercise the
# visible state we stub one of them with a plain file before
# navigating and remove it on teardown.

# Screenshot capture is OFF by default. The original PR (#2886) used
# screenshots for a one-time UX review; running them on every CI cycle
Expand Down Expand Up @@ -1921,6 +1923,32 @@ def cec_stub_device() -> Any:
pass


@pytest.fixture
def cec1_stub_device() -> Any:
"""Create a stub `/dev/cec1` so `diagnostics.cec_available()`
returns True via the second-HDMI-port path rather than
`/dev/vchiq`. Exercises the Pi 4/5 dual-micro-HDMI case where the
display is wired to the second port and only `/dev/cec1` exists
(issue #2863's actual failure mode).
"""
path = '/dev/cec1'
created = False
if not os.path.exists(path):
try:
open(path, 'w').close()
created = True
except OSError:
pytest.skip('cannot stub /dev/cec1 in this environment')
try:
yield path
finally:
if created:
try:
os.remove(path)
except FileNotFoundError:
pass


@pytest.mark.integration
@pytest.mark.django_db(transaction=True)
def test_display_power_section_hidden_without_cec_adapter(
Expand All @@ -1929,7 +1957,11 @@ def test_display_power_section_hidden_without_cec_adapter(
"""No /dev/cec0 or /dev/vchiq in the container by default — the
experimental section must NOT render. Guards against accidentally
surfacing CEC controls on x86 / non-CEC hardware."""
if os.path.exists('/dev/vchiq') or os.path.exists('/dev/cec0'):
if (
os.path.exists('/dev/vchiq')
or os.path.exists('/dev/cec0')
or os.path.exists('/dev/cec1')
):
pytest.skip('CEC device present; cannot test the hidden case')
page.goto(SETTINGS_URL)
expect(
Expand Down Expand Up @@ -1975,6 +2007,20 @@ def test_display_power_section_visible_with_cec_adapter(
)


@pytest.mark.integration
@pytest.mark.django_db(transaction=True)
def test_display_power_section_visible_with_cec1_only(
reset_assets: None, page: Page, cec1_stub_device: str
) -> None:
"""Pi 4/5 with the display on the second micro-HDMI port exposes
only /dev/cec1, not /dev/cec0 — the section must still render
instead of silently disappearing (issue #2863)."""
page.goto(SETTINGS_URL)
expect(page.get_by_role('heading', name='Display power')).to_be_visible()
expect(page.get_by_role('button', name='Turn display on')).to_be_visible()
expect(page.get_by_role('button', name='Turn display off')).to_be_visible()


@pytest.mark.integration
@pytest.mark.django_db(transaction=True)
def test_display_power_button_click_surfaces_error_toast(
Expand Down
11 changes: 11 additions & 0 deletions tests/test_diagnostics.py
Original file line number Diff line number Diff line change
Expand Up @@ -289,6 +289,17 @@ def test_cec_available_true_when_cec0_present() -> None:
assert diagnostics.cec_available() is True


def test_cec_available_true_when_only_cec1_present() -> None:
"""Pi 4/5 displays wired to the second micro-HDMI port only
expose /dev/cec1 — the gate must not assume /dev/cec0 is the only
possible CEC node (the actual failure mode behind the
device-routing fix for issue #2863)."""
with mock.patch.object(
os.path, 'exists', side_effect=lambda p: p == '/dev/cec1'
):
assert diagnostics.cec_available() is True


def test_cec_available_true_when_vchiq_present() -> None:
with mock.patch.object(
os.path, 'exists', side_effect=lambda p: p == '/dev/vchiq'
Expand Down