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: add support for HyperDeck devices #71

Merged
merged 17 commits into from Sep 26, 2022
Merged

Conversation

from-the-river-to-the-sea
Copy link
Contributor

This PR adds support for the following HyperDeck commands:

  • Play
  • Record
  • Preview

"Stop" is implicit and is defined as the absence of any of the above commands.

The other commands (Forward, Rewind, Jog, and Shuttle) really only make sense for manual control of the device and are unlikely to be supported. Fast forward and rewind functionality can already be achieved via the speed parameter of the Play command.

This branch has been tested extensively and is currently being used in a live production.

This PR will be ready to merge once nrkno/sofie-timeline-state-resolver#213 is merged and a new nightly release is available (if we're okay using a nightly release of TSR, that is).

@from-the-river-to-the-sea from-the-river-to-the-sea changed the base branch from master to develop August 26, 2022 15:16
@from-the-river-to-the-sea from-the-river-to-the-sea changed the title Feat/hyperdeck device feat: add support for HyperDeck devices Aug 26, 2022
@from-the-river-to-the-sea
Copy link
Contributor Author

The linter errors are due to TSR not being updated yet and should go away once we have an appropriate nightly.

Copy link
Member

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

Looks good!

Could you go through the list below, then I'd be happy to merge this:

  • Adress the review comment
  • Update TSR dependency to 7.3.0-nightly-release44-20220914-085659-53fbc9650.0, which contains the hyperdeck-PR
  • Test that it works and builds
  • Update README
  • Set this PR to "ready to review" and re-request a review from me :)

@from-the-river-to-the-sea
Copy link
Contributor Author

7.3.0-nightly-release44-20220914-085659-53fbc9650.0 does not appear to contain the HyperDeck PR. This may be because said PR landed on the release46 branch, not release44.

@nytamin
Copy link
Member

nytamin commented Sep 15, 2022

Ah my mistake, try this one instead: 7.4.0-nightly-release46-20220915-075618-de36c07ee.0

@from-the-river-to-the-sea
Copy link
Contributor Author

I'm not sure why, but uncaught exceptions are being thrown whenever the sideload connection fails to connect or times out. Below is an example:

[1] 2022-09-15T14:26:17.888Z [electron] info: TELEMETRY: {"reportType":"application-error","error":"Uncaught exception: Error: connect ETIMEDOUT 192.168.0.10:9993","errorStack":"Origin: uncaughtException","date":"2022-9-15","version":"0.9.1","osType":"Windows_NT","osRelease":"10.0.19044","osPlatform":"win32"}
[1] 2022-09-15T14:26:17.888Z [electron] error: Uncaught exception: Error: 
connect ETIMEDOUT 192.168.0.10:9993
[1] Origin: uncaughtException

I'm not sure where the problem lies. Is it an issue with hyperdeck-connection itself?

@sonarcloud
Copy link

sonarcloud bot commented Sep 15, 2022

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 1 Code Smell

No Coverage information No Coverage information
0.0% 0.0% Duplication

@from-the-river-to-the-sea
Copy link
Contributor Author

I'm not sure why, but uncaught exceptions are being thrown whenever the sideload connection fails to connect or times out.

Whoops, you can ignore this. I had simply forgotten to rebuild the shared packages before testing a change.

@nytamin nytamin merged commit ff6353e into develop Sep 26, 2022
@nytamin nytamin deleted the feat/hyperdeck-device branch September 26, 2022 08:58
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

2 participants