Skip to content

Conversation

@anteeek
Copy link

@anteeek anteeek commented Dec 2, 2025

About the Contributor

superfly.tv

Type of Contribution

This is a:

Feature

Current Behavior

Currently, a new device needs to be manually configured after first launch & discovery by Sofie

New Behavior

This PR adds automatic configuration of the new device, attaching it to the studio, but only if there is exactly 1 studio configured

Testing

  • I have added one or more unit tests for this PR
  • I have updated the relevant unit tests
  • No unit test changes are needed for this PR

Affected areas

This PR affects the behavior of Sofie after a new device connects for the first time

Time Frame

Not urgent, but we would like to get this merged into the in-development release

Other Information

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.

@anteeek anteeek requested a review from a team as a code owner December 2, 2025 22:15
@codecov-commenter
Copy link

codecov-commenter commented Dec 2, 2025

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

❌ Patch coverage is 90.24390% with 4 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
meteor/server/api/peripheralDevice.ts 90.24% 4 Missing ⚠️

📢 Thoughts on this report? Let us know!

@sonarqubecloud
Copy link

sonarqubecloud bot commented Dec 2, 2025

Quality Gate Failed Quality Gate failed

Failed conditions
0.0% Coverage on New Code (required ≥ 80%)

See analysis details on SonarQube Cloud


const configId = unprotectString(deviceId)

await Studios.updateAsync(studio._id, {
Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure how I feel about this creating a config object for itself when attaching..

There are 2 scenarios:

  1. blueprints provide config objects for each device. meaning the user will need to go to the settings to reassign it to the correct one and delete this temporary object
  2. blueprints done provide config objects. meaning this was helpful.

As the recommended approach for blueprints is 1, should we be prioritising that being smoother?
I dont know, Im curious what anyone else thinks

Copy link
Contributor

Choose a reason for hiding this comment

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

I too feel that this is a false friend in our "Configuration as Code" philosophy. I think we should probably re-think our "first run story": we should have the blueprints be the first thing that gets set up in a System, have them pre-populate the deviceSettings for a Studio and then, on first connection, the Peripheral Devices are matched to a known config.

Copy link
Member

Choose a reason for hiding this comment

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

Yeah I dont disagree with that, but even if blueprints are supposed to pre-generate the configurations, there will be cases where they havent so having a fallback to attach them with an empty config is still good.

So I am leaning towards leaving that larger rethinking until we have actually removed support for multiple studios; as that should in theory simplify these things even further

Copy link
Author

Choose a reason for hiding this comment

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

@Julusian @jstarpl guys, given that configIds in blueprints are arbitrary (correct me if I'm wrong) is there a way we can programmatically determine here which configId to hook up?

$push: {
[`peripheralDeviceSettings.deviceSettings.overrides`]: {
op: 'set',
path: configId,
Copy link
Member

Choose a reason for hiding this comment

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

While the risk is low, it is possible that this will replace an existing config with this empty one. I think this needs to make sure that configId isnt already defined before adding this override

@Saftret Saftret added the Contribution from SuperFly.tv Contributions sponsored by SuperFly.tv label Dec 10, 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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants