Skip to content

Always include a requested activation in the connection PATCH - #163

Open
ZayanKhan-12 wants to merge 2 commits into
sony:masterfrom
ZayanKhan-12:fix-97-include-activation-in-patch
Open

Always include a requested activation in the connection PATCH#163
ZayanKhan-12 wants to merge 2 commits into
sony:masterfrom
ZayanKhan-12:fix-97-include-activation-in-patch

Conversation

@ZayanKhan-12

Copy link
Copy Markdown

Fixes #97

Root cause

Exactly as suspected in the issue discussion: the UPDATE case in dataProvider.js builds the PATCH body from deep-diff(previousData.$staged, data.$staged). The activation object in an IS-05 PATCH is a request, not staged state — but when a receiver's staged endpoint already reports the same activation values (e.g. a node that initializes staged activation.mode to "activate_immediate", as in the reporter's setup), the diff contains no activation entries, so the Connect tab's Activate button and Staged/Edit immediate activation send a PATCH with no activation object, and nothing activates.

Minimal repro of the mechanism (deep-diff 1.0.2, same as the lockfile):

const diff = require('deep-diff');
const prev = { activation: { mode: 'activate_immediate', requested_time: null }, master_enable: true };
const next = { activation: { mode: 'activate_immediate', requested_time: null }, master_enable: true, sender_id: 'abc' };
diff(prev, next); // => [{ kind: 'N', path: ['sender_id'], rhs: 'abc' }] — activation gone

Fix

After building patchData from the differences, include data.$staged.activation whenever its mode is non-null. A null mode is deliberately not force-included: it is still only sent when it results from an actual difference, so pending scheduled activations are not unintentionally cancelled by unrelated edits.

Verification

  • yarn lint-check clean
  • yarn build succeeds (production build)
  • the repro above plus manual tracing of both affected paths (makeConnection.js Connect-tab activation and Staged/Edit) — both now always carry activation in the PATCH body when a mode is chosen

🤖 Generated with Claude Code

The PATCH body is built from the differences between the staged
endpoint response and the intended staged values, but the activation
object in a PATCH is a request rather than staged state. When a
receiver's staged endpoint already reported the same activation values
(e.g. a node that initializes staged activation.mode to
"activate_immediate"), the differences omitted the activation object
entirely, so pressing Activate in the Connect tab or activating from
Staged/Edit sent a PATCH with no activation and nothing happened.

Include the activation object whenever a mode is set, even if the
staged endpoint already reported the same values. A null mode is still
only sent when it results from an actual difference, so pending
activations are not unintentionally cancelled.

Fixes sony#97

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@garethsb

Copy link
Copy Markdown
Contributor

As pointed out in #97, the bug is in the Receiver in that case, not nmos-js, so needs further discussion about whether to include this workaround in nmos-js.

@garethsb garethsb left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think by expanding the comment slightly, the rationale is a bit clearer. Then, seems OK to me. Thanks for the contribution.

Comment thread Development/src/dataProvider.js Outdated
Comment on lines +571 to +574
// the activation object in a PATCH requests an activation, rather
// than reflecting staged state, so when a mode is set, include it
// even if the staged endpoint already reported the same values,
// which otherwise leaves it out of the differences (see #97)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
// the activation object in a PATCH requests an activation, rather
// than reflecting staged state, so when a mode is set, include it
// even if the staged endpoint already reported the same values,
// which otherwise leaves it out of the differences (see #97)
// Activation in a PATCH is a request, not staged state. Include it whenever
// the request has a non-null mode, even if GET /staged already reported the
// same values (otherwise deep-diff omits it). That also covers non-conformant
// devices that leave activate_immediate on /staged (see #97). A null mode is
// only sent when it appears in the differences, so unrelated edits do not
// cancel a pending activation.

…ed in the PATCH

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@ZayanKhan-12

Copy link
Copy Markdown
Author

Thanks for the review @garethsb — I've applied your suggested comment wording verbatim in 6a391cc. I also ran the full controller test suite locally (IS-04-04, IS-05-03 and BCP-007-03-02 via the Testing Façade against the AMWA testing tool) and all three suites pass with this change.

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.

No "activation" field in PATCH json when activating from Receivers->Staged tab or in "Connect" tab

2 participants