Skip to content
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

feat: sisyfos enhancements #324

Draft
wants to merge 20 commits into
base: release51
Choose a base branch
from

Conversation

olzzon
Copy link
Contributor

@olzzon olzzon commented Mar 21, 2024

About the Contributor

This PR is posted on behalf of BBC

Type of Contribution

This is a:
Feature request

Bug fix / Feature / Code improvement / Documentation improvement / Other (please specify)

Old Behavior:

Sisyfos and TSR had some limitation in the API, regarding mute, inputGain & inputSelector.

New Behavior

TSR and Sisyfos has been enhanced to include support for optional mute, inputgain, inputselector and get sisyfos state of a fader.
Following changes has been made:

  • Added:
    • setMutoOn, setInputgain, setInputSelector in:
      • SisyfosChannelOptions in TSR types sisyfos integration
      • API types SisyfosSendChannelAPI and SisyfosReceiveChannelAPI - with new features as optionals.
      • SisyfosCommandType as SET_INPUT_SELECTOR, SET_INPUT_GAIN and SET_MUTE
    • 2 new actions are added:
      • ReSyncChannel - the ability to resync a channel from Sisyfos without reconnection (as a full resin/reinit does)
      • SetSisyfosChannelState - works like triggerValue but as an action
    • Updated the receiver() to include receiving a single channel from Sifyfos
  • Refactor:
    • the SET_CHANNEL command shares setSisyfosChannel() with the SetSisyfosChannelState action.

Fixes:

  • quickTSR has been update to check if a timeline refers to an unmapped object.

Testing Instructions

use quickTSR to test the new implementations.
All features implemented as optional, so it don't have breaking changes in the current TSR-Sisyfos behavior.

Other Information

Status

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

@nytamin nytamin requested a review from mint-dewit March 21, 2024 10:14
@@ -6,6 +6,38 @@
"name": "Reinitialize",
"destructive": false,
"timeout": 5000
},
{
"id": "reSyncChannel",
Copy link
Member

Choose a reason for hiding this comment

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

As far as I can tell this action does not do much from a user perspective. It will pull the external state from Sisyfos but as this state is not used to compare it to the timeline I believe it effectively will not send any commands?

I think anyone using this action will expect it to do what setSisyfosChannelState and we are better off keeping only that. Would love to hear your thoughts on that and maybe I'm just understanding this wrong.

@jstarpl jstarpl changed the title Feat/sisyfos bbc enhancements feat: sisyfos enhancements Mar 25, 2024
@codecov-commenter
Copy link

codecov-commenter commented Apr 2, 2024

Codecov Report

Attention: Patch coverage is 25.67568% with 55 lines in your changes are missing coverage. Please review.

Project coverage is 53.46%. Comparing base (b7ceb69) to head (3cb6f70).

Files Patch % Lines
...te-resolver/src/integrations/sisyfos/connection.ts 34.69% 28 Missing and 4 partials ⚠️
...e-state-resolver/src/integrations/sisyfos/index.ts 0.00% 19 Missing and 4 partials ⚠️
Additional details and impacted files
@@              Coverage Diff              @@
##           release51     #324      +/-   ##
=============================================
- Coverage      53.59%   53.46%   -0.14%     
=============================================
  Files            125      125              
  Lines          10129    10172      +43     
  Branches        2358     2510     +152     
=============================================
+ Hits            5429     5438       +9     
+ Misses          4698     4347     -351     
- Partials           2      387     +385     

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

@olzzon
Copy link
Contributor Author

olzzon commented Apr 3, 2024

Following bugs has been fixed:

  • Channel index/number issues between internal (index) and OSC (channel number)
  • When receiving Sisyfos' current state from ReSyncChannel it didn't set the State in TSR
  • Backward compatibility between Sisyfos using showChannel and TSR using visible.

@mint-dewit do you have time for a look?

@nytamin nytamin requested a review from mint-dewit April 3, 2024 07:02
@olzzon olzzon marked this pull request as draft April 4, 2024 07:36
@nytamin nytamin removed the request for review from mint-dewit April 4, 2024 07:36
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.

None yet

3 participants