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

Add a NodeId header and some NodeId utilities #8465

Merged
merged 1 commit into from
Jul 21, 2021

Conversation

bzbarsky-apple
Copy link
Contributor

Fixes #8461

The encoder.cpp changes are needed because it used to bootleg the
PacketBuffer header via basic-types.h including MessageHeader.h,
instead of doing include-what-you-use correctly.

Problem

Could use a utility to generate spec-valid random operational node ids.

Change overview

Add such a utility.

Testing

Made sure that chip-tool is still generating random node ids when it should.

@todo
Copy link

todo bot commented Jul 17, 2021

Consider making this a class and the various utility methods static

// TODO: Consider making this a class and the various utility methods static
// methods.
using NodeId = uint64_t;
constexpr NodeId kUndefinedNodeId = 0ULL;
constexpr NodeId kAnyNodeId = 0xFFFF'FFFF'FFFF'FFFFULL;
constexpr bool IsOperationalNodeId(NodeId aNodeId)
{
return (aNodeId != kUndefinedNodeId) && (aNodeId <= 0xFFFF'FFEF'FFFF'FFFFUL);
}


This comment was generated by todo based on a TODO comment in 2de562c in #8465. cc @bzbarsky-apple.

src/lib/core/NodeId.h Outdated Show resolved Hide resolved
@bzbarsky-apple
Copy link
Contributor Author

@msandstedt @tcarmelveilleux Please take a look?

@bzbarsky-apple bzbarsky-apple force-pushed the util-for-node-id branch 2 times, most recently from 5683431 to a11c2f4 Compare July 20, 2021 03:41
@github-actions
Copy link

Size increase report for "esp32-example-build" from d24b3c0

File Section File VM
chip-lock-app.elf .flash.text -64 -64
Full report output
BLOAT REPORT

Files found only in the build output:
    report.csv

Comparing ./master_artifact/chip-temperature-measurement-app.elf and ./pull_artifact/chip-temperature-measurement-app.elf:

sections,vmsize,filesize
.debug_info,0,8552
.debug_str,0,177
.debug_loc,0,-4
.debug_abbrev,0,-2884
.debug_line,0,-3841

Comparing ./master_artifact/chip-all-clusters-app.elf and ./pull_artifact/chip-all-clusters-app.elf:

sections,vmsize,filesize
.debug_info,0,15107
.debug_str,0,176
.debug_loc,0,16
.riscv.attributes,0,1
.debug_abbrev,0,-2883
.debug_line,0,-3573

Comparing ./master_artifact/chip-persistent-storage.elf and ./pull_artifact/chip-persistent-storage.elf:

sections,vmsize,filesize

Comparing ./master_artifact/chip-pigweed-app.elf and ./pull_artifact/chip-pigweed-app.elf:

sections,vmsize,filesize

Comparing ./master_artifact/chip-shell.elf and ./pull_artifact/chip-shell.elf:

sections,vmsize,filesize
.debug_info,0,5551
.debug_line,0,264
.debug_str,0,176
.debug_loc,0,17

Comparing ./master_artifact/chip-ipv6only-app.elf and ./pull_artifact/chip-ipv6only-app.elf:

sections,vmsize,filesize

Comparing ./master_artifact/chip-lock-app.elf and ./pull_artifact/chip-lock-app.elf:

sections,vmsize,filesize
.debug_str,0,174
[Unmapped],0,64
.debug_loc,0,3
.flash.text,-64,-64
.debug_line,0,-4714
.debug_abbrev,0,-6038
.debug_info,0,-48453


src/controller/CHIPDeviceController.cpp Outdated Show resolved Hide resolved
@todo
Copy link

todo bot commented Jul 20, 2021

Ideally we'd just ask for an operational cert for the commissionnee

// TODO: Ideally we'd just ask for an operational cert for the commissionnee
// and get the node from that, but the APIs are not set up that way yet.
NodeId randomId;
if (Controller::ExampleOperationalCredentialsIssuer::GetRandomOperationalNodeId(&randomId) == CHIP_NO_ERROR)
{
ChipLogProgress(Controller, "Generated random node id: 0x" ChipLogFormatX64, ChipLogValueX64(randomId));
if (GetExecContext()->storage->SetRemoteNodeId(randomId) == CHIP_NO_ERROR)
{
GetExecContext()->remoteId = randomId;
}
}


This comment was generated by todo based on a TODO comment in ada92cf in #8465. cc @bzbarsky-apple.

Fixes project-chip#8461

The encoder.cpp changes are needed because it used to bootleg the
PacketBuffer header via basic-types.h including MessageHeader.h,
instead of doing include-what-you-use correctly.
@todo
Copy link

todo bot commented Jul 20, 2021

Ideally we'd just ask for an operational cert for the commissionnee

// TODO: Ideally we'd just ask for an operational cert for the commissionnee
// and get the node from that, but the APIs are not set up that way yet.
NodeId randomId;
if (Controller::ExampleOperationalCredentialsIssuer::GetRandomOperationalNodeId(&randomId) == CHIP_NO_ERROR)
{
ChipLogProgress(Controller, "Generated random node id: 0x" ChipLogFormatX64, ChipLogValueX64(randomId));
if (GetExecContext()->storage->SetRemoteNodeId(randomId) == CHIP_NO_ERROR)
{
GetExecContext()->remoteId = randomId;
}
}


This comment was generated by todo based on a TODO comment in af22b22 in #8465. cc @bzbarsky-apple.

@bzbarsky-apple
Copy link
Contributor Author

@mspang mspang merged commit fd0d32d into project-chip:master Jul 21, 2021
@bzbarsky-apple bzbarsky-apple deleted the util-for-node-id branch July 21, 2021 22:29
nikita-s-wrk pushed a commit to nikita-s-wrk/connectedhomeip that referenced this pull request Sep 23, 2021
Fixes project-chip#8461

The encoder.cpp changes are needed because it used to bootleg the
PacketBuffer header via basic-types.h including MessageHeader.h,
instead of doing include-what-you-use correctly.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add a GenerateNodeId utility
8 participants