Feat: WebSocket Client #369
Conversation
…commands Co-authored-by: Johan Nyman <[email protected]>
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## release52 #369 +/- ##
=============================================
- Coverage 57.22% 57.12% -0.10%
=============================================
Files 160 164 +4
Lines 11016 11121 +105
Branches 2686 2707 +21
=============================================
+ Hits 6304 6353 +49
- Misses 4323 4374 +51
- Partials 389 394 +5 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
In general it seems this PR hasn't been linted and formatted. It is recommended to have automatic formatting on save enabled in the Sofie projects, and commit hooks to fail when the linting doesn't pass on commit.
I am quite confused as to why an additional device integration is being added for TCP connections when we already have the TCPSendDevice. There appears to be no benefit to having the TCP connection be an option in this integration.
packages/timeline-state-resolver/src/integrations/websocketTcpClient/index.ts
Outdated
Show resolved
Hide resolved
packages/timeline-state-resolver/src/integrations/websocketTcpClient/index.ts
Outdated
Show resolved
Hide resolved
| [WebsocketTcpClientActions.ResetState]: async (_id: string) => { | ||
| return { result: ActionExecutionResultCode.Ok } | ||
| }, | ||
| [WebsocketTcpClientActions.SendWebSocketMessage]: async (_id: string, payload?: Record<string, any>) => { |
There was a problem hiding this comment.
The typings for the payload are wrong, as per the actions.json it should be
| [WebsocketTcpClientActions.SendWebSocketMessage]: async (_id: string, payload?: Record<string, any>) => { | |
| [WebsocketTcpClientActions.SendWebSocketMessage]: async (_id: string, payload?: SendWebSocketMessagePayload) => { |
There was a problem hiding this comment.
Maybe I've misunderstood something, but from looking through the code in the other integrations, it does look like the actions payload need to be a Record<string, xxx> type
So I changed the type from any it to:
payload?: Record<string, SendWebSocketMessagePayload>
And the iterate over the Record and send the message directly to the connection.
packages/timeline-state-resolver/src/integrations/websocketTcpClient/index.ts
Outdated
Show resolved
Hide resolved
packages/timeline-state-resolver/src/integrations/websocketTcpClient/index.ts
Outdated
Show resolved
Hide resolved
packages/timeline-state-resolver/src/integrations/websocketTcpClient/$schemas/options.json
Outdated
Show resolved
Hide resolved
packages/timeline-state-resolver/src/integrations/websocketTcpClient/$schemas/options.json
Outdated
Show resolved
Hide resolved
| afterEach(() => { | ||
| //Are there something like??: | ||
| //mockTime.dispose() | ||
| // Or can we just ignore this |
There was a problem hiding this comment.
You might want to clear the mocks here and reset the timeMock
| }) | ||
|
|
||
| test('terminate', async () => { | ||
| await device.terminate() |
There was a problem hiding this comment.
This could cause race conditions when this test is ran in parallel with other tests
About the Contributor
This is made on behalf of BBC
Type of Contribution
This is a feature and code comment improvements
Current Behavior
New Behavior
This adds support for a general WS socket device
Testing Instructions
Other Information
Status