Skip to content

feat!: support loading TSR devices as 'plugins' #375

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 2 commits into
base: release53
Choose a base branch
from

Conversation

Julusian
Copy link
Member

@Julusian Julusian commented Apr 22, 2025

About the Contributor

This pull request is posted on behalf of Superfly

Type of Contribution

This is a: Feature

Current Behavior

All the device integrations must be built into this repository. This sometimes means an organisation must make a private fork of this repository, when they have an integration which they are not allowed to make public (typically because a vendor restricts access to protocol docs)

New Behavior

This makes it possible to load extra device integrations from additional paths on disk.

This loading is currently a little naive and isn't attempting to check that the api is the same version as expected, or doing much sanitisation of the manifest or other data. Due to the limited scope of how to load these plugins, I suspect it will be acceptable to say that is not necessary.

To use this a folder should be constructed meeting the following requrements:

  • Have a manifest.json at the root of it, containing json in the format of Record<string, TSRDevicesManifestEntry>
  • Have a translations.json at the root of it, containing json in the format of TranslationsBundle[]
  • Be importable from nodejs with require(...). This likely means having a package.json, be a commonjs project and having all necessary dependencies installed within the folder.
  • Importing the folder with require(...) should have an export of the form export const Devices: Record<string, DeviceEntry>

Then in the code using TSR, this can be loaded by:

const devicesRegistry = new DevicesRegistry()

await devicesRegistry.loadDeviceIntegrationsFromPath('/some/path/here')

const tsr = new Conductor({
	devicesRegistry,

	... // other options here
})

It is possible to load extra devices on the DevicesRegistry while it is being used by a Conductor if needed, but it isn't possible to unload plugin paths currently, so this can 'leak' if used excessively.

I opted to make this a separate DevicesRegistry class because playout-gateway needs access to the full manifest before the Conductor has been constructed. I also wanted to avoid adding any global state. Providing this class to the Conductor is optional, so it does not add complexity for those who do not want to make use of it.
As a side effect of this change, the static manifest export has been removed, making this a breaking change.

To achieve this, a new timeline-state-resolver-api package has been added which contains the interfaces, schemas and other common code needed by these plugins and tsr itself.

A timeline-state-resolver-tools package has also been added which contains a few scripts to generate and prepare translations, and process the device integration json-schemas. These scripts are refactored versions of ones that used to be part of the timeline-state-resolver package, and have been made a bit more generic to be reusable by plugins and tsr.
There is an annoying quirk of this library, users of it must use the following yarn resolution for it to work properly "cheerio": "1.0.0-rc.12". This is a known bug in the version of i18next-parser we use. It is fixed in the latest version of that library, which requires node 18 or later, and might require us to update i18next everywhere.

Testing Instructions

This can be tested with quick-tsr, there is a commented out call to loadDeviceIntegrationsFromPath(), which can be enabled and given a correct path. To utilise it with sofie, the patch Sofie-Automation/sofie-core@release53...SuperFlyTV:sofie-core:feat/tsr-plugins can be used

Other Information

My testing plugin for this is https://github.com/SuperFlyTV/sofie-tsr-plugin-example, which is the atem module pulled out of this repository. Only imports (and tooling) needed to be updated to get the module running.

I haven't verified that the types in sofie blueprints or elsewhere work for this, I suspect there may be some fighting with TSR complaining that strings arent assignable to DeviceType

Status

  • PR is ready to be reviewed.
  • The functionality has been tested by the author.
  • Relevant unit tests has been added / updated.
  • Relevant documentation (code comments, system documentation) has been added / updated.

@codecov-commenter
Copy link

codecov-commenter commented Apr 22, 2025

Codecov Report

Attention: Patch coverage is 58.69565% with 38 lines in your changes missing coverage. Please review.

Project coverage is 58.62%. Comparing base (e732d93) to head (bd2cd20).

Files with missing lines Patch % Lines
...line-state-resolver/src/service/devicesRegistry.ts 38.23% 21 Missing ⚠️
...eline-state-resolver/src/service/DeviceInstance.ts 45.00% 11 Missing ⚠️
...ne-state-resolver/src/service/ConnectionManager.ts 79.31% 6 Missing ⚠️
Additional details and impacted files
@@              Coverage Diff              @@
##           release53     #375      +/-   ##
=============================================
- Coverage      58.69%   58.62%   -0.07%     
=============================================
  Files            166      167       +1     
  Lines          11185    11239      +54     
  Branches        2728     2736       +8     
=============================================
+ Hits            6565     6589      +24     
- Misses          4616     4648      +32     
+ Partials           4        2       -2     

☔ View full report in Codecov by Sentry.
📢 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.

@Julusian Julusian force-pushed the feat/devices-as-plugins branch from e6e3d75 to 15c174c Compare April 23, 2025 10:07
@Julusian Julusian force-pushed the feat/devices-as-plugins branch from 15c174c to bd2cd20 Compare April 23, 2025 10:09
@Julusian Julusian marked this pull request as ready for review May 7, 2025 09:44
@Julusian Julusian requested a review from a team as a code owner May 7, 2025 09:44
@jstarpl jstarpl added the contribution from SuperFly.tv Contributions sponsored by SuperFly.tv label May 7, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
contribution from SuperFly.tv Contributions sponsored by SuperFly.tv
Development

Successfully merging this pull request may close these issues.

3 participants