Skip to content

tv-app: select media device type at runtime (advertised + declared) (backport #73895) - #74070

Open
mergify[bot] wants to merge 1 commit into
v1.7-branchfrom
mergify/bp/v1.7-branch/pr-73895
Open

tv-app: select media device type at runtime (advertised + declared) (backport #73895)#74070
mergify[bot] wants to merge 1 commit into
v1.7-branchfrom
mergify/bp/v1.7-branch/pr-73895

Conversation

@mergify

@mergify mergify Bot commented Sep 10, 2026

Copy link
Copy Markdown
Contributor

Problem

tv-app is hard-wired to the Casting Video Player (0x0023) device type in two independent places that must be kept in agreement by hand — the compile-time CHIP_DEVICE_CONFIG_DEVICE_TYPE (DNS-SD advertising) and endpoint 1's declaration in tv-app.zap/tv-app.matter (Descriptor DeviceTypeList). There is no supported, documented way to run it as any of the other media player device types, even though the spec now defines four: Basic Video Player (0x0028), Casting Video Player (0x0023), Casting Audio Player (0x0021) and Streaming Audio Player (0x0020).

Change

Two commits:

1. platform: allow overriding the advertised device type id at runtime. GetDeviceTypeId() returned CHIP_DEVICE_CONFIG_DEVICE_TYPE unconditionally, fixing the DNS-SD _T<id> subtype at compile time. Add a SetDeviceTypeId() setter on ConfigurationManager, backed by a non-persistent in-RAM override in GenericConfigurationManagerImpl. This mirrors the existing Get/SetFirmwareBuildChipEpochTime override: the interface method defaults to CHIP_ERROR_NOT_IMPLEMENTED (no platform impl is forced to change), and GetDeviceTypeId() falls back to the compile-time value when unset. Dnssd.cpp already sources the advertised type from GetDeviceTypeId(), so advertising follows automatically. Unit test added in TestConfigurationMgr.

2. tv-app: add --device-type flag. Endpoint 1 already hosts a superset of clusters that satisfies the mandatory set of all four media player device types. The flag --device-type <casting-video|basic-video|casting-audio|streaming-audio> presents the app as any of the four without a rebuild:

  • during argument parsing (before the server advertises), it overrides the DNS-SD _T subtype via ConfigurationMgr().SetDeviceTypeId;
  • in ApplicationInit, it rewrites endpoint 1's Descriptor DeviceTypeList via emberAfSetDeviceTypeList.

Default behavior (Casting Video Player) is unchanged.

Scope / caveats (documented, not worked around)

The flag changes the advertised and declared device type. It intentionally does not change the commissioner role (Casting players are Commissioners; Basic/Streaming are commissionable-only — that machinery is compiled in) or trim the cluster set. A new examples/tv-app/README.md documents the flag and these caveats, and describes the build-time variant path (per-type ZAP/.matter) for a fully faithful data model; examples/tv-app/linux/README.md links to it.

Testing

…73895)

* platform: allow overriding the advertised device type id at runtime

GetDeviceTypeId() returned CHIP_DEVICE_CONFIG_DEVICE_TYPE unconditionally, so
the DNS-SD "_T<id>" commissioning subtype was fixed at compile time. Add a
SetDeviceTypeId() setter on ConfigurationManager, backed by a non-persistent
in-RAM override in GenericConfigurationManagerImpl, so an application can change
the advertised device type at boot. This mirrors the existing
Get/SetFirmwareBuildChipEpochTime override: the interface method defaults to
CHIP_ERROR_NOT_IMPLEMENTED so no platform implementation is forced to change,
and GetDeviceTypeId() falls back to the compile-time value when no override is
set.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* tv-app: add --device-type flag to select media device type at runtime

tv-app is built as a Casting Video Player (0x0023), with endpoint 1 hosting a
superset of clusters that also covers the mandatory set of the other three
media player device types. Add a `--device-type` flag
(casting-video|basic-video|casting-audio|streaming-audio) that presents the app
as any of the four without a rebuild:

- during argument parsing (before the server starts advertising), it overrides
  the DNS-SD "_T<id>" commissioning subtype via ConfigurationMgr().SetDeviceTypeId;
- in ApplicationInit, it rewrites endpoint 1's Descriptor DeviceTypeList via
  emberAfSetDeviceTypeList.

The commissioner role and cluster set are as compiled; document both the
runtime flag and that caveat, plus the build-time variant path for a fully
faithful data model, in a new examples/tv-app/README.md linked from
linux/README.md.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* tv-app: address review feedback on --device-type

- Use the generated devices/Types.h entries (kCastingVideoPlayer,
  kBasicVideoPlayer) instead of hardcoded ids for the video player types, so
  they track the data model. The two audio player types are not yet emitted
  there (absent from matter-devices.xml) and remain spelled out.
- Fix the docs build: add the top-level tv-app README to the Sphinx toctree
  (the tv-app/**/README glob only matches nested READMEs).
- Fix the spellcheck: add "superset" to the CI wordlist.
- Fix markdownlint MD040: tag the fenced command block as sh.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* platform: honor device-type override in ESP32/android GetDeviceTypeId

The runtime device-type override (SetDeviceTypeId) was only consulted by the
generic GetDeviceTypeId used on Linux/darwin. ESP32 and android provide their
own GetDeviceTypeId that reads persisted config, so they ignored the override:
after SetDeviceTypeId() the getter still returned the stored/compile-time value.
This surfaced as a TestConfigurationMgr.DeviceTypeId failure on the ESP32 QEMU
test (returned 0 instead of the value just set).

Have those platform getters consult the override first via a small
GetDeviceTypeIdOverride() accessor, and move the override storage from a
file-static in the .ipp to a static data member of the template. A static data
member is a single external-linkage instance per specialization, so the setter
and every platform getter observe the same object regardless of which
translation unit instantiates them.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* tv-app: only record device-type override after SetDeviceTypeId succeeds

Set gMediaDeviceTypeList/gMediaDeviceTypeOverridden only once
SetDeviceTypeId() has succeeded, and fail the option otherwise, so a failed
advertised-type override does not leave the app claiming an override (and
rewriting the Descriptor DeviceTypeList in ApplicationInit) that never took
effect.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
(cherry picked from commit 4592828)
@mergify mergify Bot added the backport-v1.7-branch PR that is a backport from master to the v1.7-branch development branch label Sep 10, 2026
@coderabbitai

coderabbitai Bot commented Sep 10, 2026

Copy link
Copy Markdown
Contributor

Important

Review skipped

Bot user detected.

To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Advanced

Run ID: 160ec32b-3ba5-4cc2-969f-ac0aa6d54db8

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@github-actions github-actions Bot added documentation Improvements or additions to documentation examples platform esp32 github labels Sep 10, 2026
@codecov

codecov Bot commented Sep 10, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 80.00000% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 56.42%. Comparing base (804aa60) to head (8b7c240).
⚠️ Report is 1 commits behind head on v1.7-branch.

Files with missing lines Patch % Lines
src/include/platform/ConfigurationManager.h 0.00% 1 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##           v1.7-branch   #74070   +/-   ##
============================================
  Coverage        56.42%   56.42%           
============================================
  Files             1862     1862           
  Lines           121963   121967    +4     
  Branches         14352    14352           
============================================
+ Hits             68816    68819    +3     
- Misses           53147    53148    +1     

☔ 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.

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

Labels

backport-v1.7-branch PR that is a backport from master to the v1.7-branch development branch documentation Improvements or additions to documentation esp32 examples github platform

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants