Quantel: reimplement clearAllWaitWithPort #352
Conversation
…multiple times. The commands should be executed in the order that they are given.
This is to ensure that all commands are executed in ssequential order, if calling .executeCommands() multiple times.
…/quantel-cancelled-outTransition
…o/tv-automation-state-timeline-resolver into fix/quantel-cancelled-outTransition
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## release52 #352 +/- ##
=============================================
+ Coverage 56.60% 57.29% +0.69%
=============================================
Files 131 131
Lines 10265 10307 +42
Branches 2503 2573 +70
=============================================
+ Hits 5810 5905 +95
+ Misses 4453 4041 -412
- Partials 2 361 +359 ☔ View full report in Codecov by Sentry. |
| // * When the commands sent into executeCommands() is called a BATCH. | ||
| // | ||
| // * Salvos should wait for Salvos from previous BATCH to have finished before executing | ||
| // * Salvos within the same BATCH are executed in parallel. | ||
| // | ||
| // * Sequentials should wait for all Salvos and Sequentials from previous BATCH to have finished before executing | ||
| // * Sequentials should be executed after all Salvos from the same BATCH have been executed | ||
| // * Sequentials within the same BATCH are executed in order |
There was a problem hiding this comment.
I am somewhat worried about this algorithm mixing commands from different batches. After all, Salvos from Batch N+1 have been generated assuming that Salvos and Sequentials from Batch N have already been sent. I feel like each batch should be done, before we start sending commands for the next batch? They are effectively applying state, and thus each batch depends on the previous one to have been applied?
There was a problem hiding this comment.
I agree with Jan, this feels a bit dangerous. But I don't think the behaviour from the R50 Quantel Integration can be replicated without this because the next state change has to abort the previous one.
1 question I have is whether this "wait until the state change has completed" should be the responsibility of the CommandExecutor or the StateHandler. The StateHandler knows at what time a state should be active. If I have 5 states with 200ms inbetween but each change takes 2 seconds to execute perhaps we should skip the states inbetween and transition from first to last state? (By recalculating how to get into that last state of course)
There was a problem hiding this comment.
Perhaps a solution to this is to write some really good documentation about the intention of the type of commands?
For my use case, I would need it to work this way, because:
- Batch 1:
- Sequntial: Quantel stop command (delayed, so will wait and not resolve until after an
await sleep(x))
- Sequntial: Quantel stop command (delayed, so will wait and not resolve until after an
- Batch 2:
- Salvo: cancel any waiting Quantel operations
- Sequntial: Quantel play command
So in this case, the cancel-salvo must not wait for the previous batch to finish.
The only other possible solution to this would be to handle specific situations like this internally in the device - but that goes agains the idea of the rewrite of the TSR devices where we try to unify the integration..
| cmd.command.type === QuantelCommandType.PAUSECLIP || | ||
| cmd.command.type === QuantelCommandType.CLEARCLIP || | ||
| cmd.command.type === QuantelCommandType.RELEASEPORT) && | ||
| !cmd.command.fromLookahead |
There was a problem hiding this comment.
I think this should be looking at the port.notOnAir property instead like it does in
However I can see that in the original release50 implementation this wasn't the case either (but it wasn't triggering for clear and release there either)
About the Contributor
This pull request is posted on behalf of the NRK.
Type of Contribution
This is a: Bug/functionality fix
Current Behavior
There currently is a situation where:
so the stop command wont be sent until a while later
The Quantel device might either
New Behavior
This PR has multiple fixes.
First, the funcitonality of
CommandExecutorhas been rewritten to support a mix of SALVO and SEQUENTIAL commands.Secondly, the Quantel device has now added a CLEARWAITING command (of type SALVO) that is exececuted before any play, stop, clear, ect, command, to ensure that a delayed command is cancelled, whatever it was going to do will be replaced by the new command.
Testing Instructions
Other Information
Status