Skip to content

Conversation

@AlvaroVega
Copy link
Member

@AlvaroVega AlvaroVega commented Aug 28, 2025

Issue: #1732

  • Doc
  • Tests
  • Test device and group config

@fgalan
Copy link
Member

fgalan commented Aug 29, 2025

Maybe useCmdBySubs should use a string value instead of true/false to make it more extensible? I mean:

@AlvaroVega AlvaroVega changed the title Task/use cmd by subs allow use commands by simple subscriptions/notifications Sep 1, 2025
@AlvaroVega AlvaroVega marked this pull request as ready for review September 2, 2025 08:20
@AlvaroVega AlvaroVega requested a review from fgalan September 2, 2025 08:20
}

function updateRegisterDevice(deviceObj, previousDevice, entityInfoUpdated, callback) {
//
Copy link
Member

Choose a reason for hiding this comment

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

Leftover? Or incomplete comment?

Copy link
Member Author

Choose a reason for hiding this comment

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

Done in 93c8902

apply(extractDeviceDifference, deviceObj),
createInitialEntityNgsi2Fake,
apply(combineWithNewDevice, deviceObj),
//
Copy link
Member

Choose a reason for hiding this comment

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

Leftover? Or incomplete comment?

Copy link
Member Author

Choose a reason for hiding this comment

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

Done in 93c8902

.reduce(mergeWithSameName, []);

if (attrs.length === 0) {
logger.debug(context, 'Subscription is not needed. Device without lazy attrs or commands');
Copy link
Member

Choose a reason for hiding this comment

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

Not sure if the references to "lazy attrs" is correct here... The subscription will only cover the commands case, not lazy attributes.

config.storeLastMeasure = config.storeLastMeasure === true;
}
if (process.env.IOTA_CMD_MODE) {
config.cmdMode = process.env.IOTA_CMD_MODE;
Copy link
Member

Choose a reason for hiding this comment

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

Should the new config item / env var be described in some .md file? Not sure, but I think to remember we have a document about configurations...

Copy link
Member Author

Choose a reason for hiding this comment

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

Done in 93c8902

@fgalan
Copy link
Member

fgalan commented Sep 2, 2025

Coverage show s -0.2% decrease. Although I have seen some test has been modified to align with the new implementation, is the new logic covered by new tests?

@fgalan
Copy link
Member

fgalan commented Sep 2, 2025

CNR entry should be added describing the changes in this PR (new fields added, basically)

@AlvaroVega
Copy link
Member Author

Still work in progress

@AlvaroVega
Copy link
Member Author

@fgalan this PR (and related PRs) is ready

@AlvaroVega
Copy link
Member Author

AlvaroVega commented Oct 1, 2025

  • Ensure related entity is not created when device has cmdMode notification but none commands are created

Clarified entity creation conditions regarding commands and attributes.
AlvaroVega and others added 3 commits October 2, 2025 09:04
Copy link
Member

@fgalan fgalan left a comment

Choose a reason for hiding this comment

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

LGTM

@fgalan fgalan merged commit d25d80f into master Oct 2, 2025
9 checks passed
@fgalan fgalan deleted the task/use_cmd_by_subs branch October 2, 2025 09:33
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.

3 participants