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: implement VISCA over IP and Panasonic PTZ actions #340

Merged
merged 12 commits into from
Nov 28, 2024

Conversation

ianshade
Copy link
Contributor

@ianshade ianshade commented Jul 5, 2024

About the Contributor

This PR is opened on behalf of TV 2 Norge.

Type of Contribution

This is a:

Feature (including some Code improvements)

Current Behavior

  • No Actions on Panasonic PTZ integration
  • No VISCA over IP integration at all

New Behavior

  • PTZ actions are added to the Panasonic PTZ integration
  • VISCA over IP integration is added, featuring just PTZ actions

Testing Instructions

Other Information

VISCA over IP integration is based on a fixed and refactored version of https://github.com/nrkno/sofie-sony-visca-connection

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 Jul 5, 2024

Codecov Report

Attention: Patch coverage is 64.79903% with 289 lines in your changes missing coverage. Please review.

Project coverage is 57.12%. Comparing base (3e5de55) to head (9c024ae).

Files with missing lines Patch % Lines
.../integrations/viscaOverIP/connection/lib/socket.ts 4.13% 102 Missing and 14 partials ⚠️
...te-resolver/src/integrations/panasonicPTZ/index.ts 6.57% 70 Missing and 1 partial ⚠️
...ate-resolver/src/integrations/viscaOverIP/index.ts 18.18% 63 Missing ⚠️
...r/src/integrations/viscaOverIP/connection/visca.ts 13.63% 18 Missing and 1 partial ⚠️
.../viscaOverIP/connection/lib/ViscaValueConverter.ts 93.65% 4 Missing ⚠️
...solver/src/integrations/panasonicPTZ/connection.ts 80.00% 3 Missing ⚠️
...nnection/commands/control/resetSeqNumberCommand.ts 50.00% 2 Missing ⚠️
...ection/commands/inquiry/focusModeInquiryCommand.ts 66.66% 2 Missing ⚠️
...on/commands/inquiry/focusPositionInquiryCommand.ts 71.42% 2 Missing ⚠️
.../commands/inquiry/panTiltPositionInquiryCommand.ts 75.00% 2 Missing ⚠️
... and 3 more
Additional details and impacted files
@@              Coverage Diff              @@
##           release52     #340      +/-   ##
=============================================
+ Coverage      56.55%   57.12%   +0.57%     
=============================================
  Files            131      160      +29     
  Lines          10274    10908     +634     
  Branches        2509     2657     +148     
=============================================
+ Hits            5810     6231     +421     
+ Misses          4462     4291     -171     
- Partials           2      386     +384     

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

@jstarpl jstarpl added the contribution from TV 2 Norge Contributions sponsored by TV 2 Norge (tv2.no) label Sep 20, 2024
@ianshade ianshade changed the base branch from release51 to release52 November 26, 2024 18:36
@ianshade ianshade marked this pull request as ready for review November 26, 2024 18:45
@ianshade ianshade requested a review from a team as a code owner November 26, 2024 18:45
Copy link
Contributor

@nytamin nytamin left a comment

Choose a reason for hiding this comment

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

Implementation looks good! This is going to be a great addition!

One note for the future: Maybe we should think about (and write documentation for) whether executing an action should affect the internal state or not.

@nytamin nytamin merged commit f95110c into nrkno:release52 Nov 28, 2024
17 of 18 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
contribution from TV 2 Norge Contributions sponsored by TV 2 Norge (tv2.no)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants