Skip to content

Send current IP address during network provisioning #4591

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Feb 2, 2021
Merged

Send current IP address during network provisioning #4591

merged 1 commit into from
Feb 2, 2021

Conversation

pan-apple
Copy link
Contributor

@pan-apple pan-apple commented Feb 1, 2021 •
edited
Loading

Problem

For multi-admin scenarios, the device address needs to be detected by the additional commissioners. In the current flow, the device sends its IP address to the commissioner as part of the network provisioning flow. Eventually, it'll be changed to DNS-SD.

In current flow, the network provisioning state machine doesn't send the IP address to the commissioner, if it was already connected to the network. The state machine needs to be updated, so that it can detect it has a valid IP, and send it to the commissioner.

Summary of Changes

When network provisioning command is received from the commissioner, the device will

  1. check if it is already connected to the network
  2. find the interface that's up, is not loopback, and is not an access point (in case it was configured)
  3. get the IP address of the interface, and send it to the commissioner.

Note

This is a temporary solution. Once Network Provisioning cluster and DNS-SD is implemented/integrated, the NetworkProvisioning.cpp/.h class implementation will be removed. This change is contained within this class, so it'll be removed along with it.

@github-actions
Copy link

github-actions bot commented Feb 1, 2021

Size increase report for "esp32-example-build" from 1073ccf

File Section File VM
chip-all-clusters-app.elf .flash.text 224 224
chip-all-clusters-app.elf .flash.rodata 8 8
Full report output 
BLOAT REPORT

Files found only in the build output:
    report.csv

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

sections,vmsize,filesize
.debug_info,0,13941
.debug_line,0,809
.debug_loc,0,276
.debug_abbrev,0,263
.strtab,0,240
.flash.text,224,224
.debug_str,0,155
.symtab,0,80
.debug_ranges,0,48
.debug_frame,0,24
.debug_aranges,0,8
.flash.rodata,8,8
[Unmapped],0,-8

andy31415
andy31415 reviewed Feb 2, 2021
View reviewed changes
src/transport/NetworkProvisioning.h
* The device can use this function to send its current IP address to
* commissioner. This would generally be called during network
* provisioning of the device, when the device already has an IP address.
*/
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we add a GH issue to change this?
If this is to stay, the comment should explain why we are specifically using IPv4 when CHIP is supposed to work with IPv6.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@andy31415, this code NetworkProvisioning.h/.cpp will be deleted once we move the provisioning to ZCL cluster. I'll file a GH issue in regards to that.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Filed #4606

andy31415
andy31415 reviewed Feb 2, 2021
View reviewed changes
src/transport/NetworkProvisioning.cpp

CHIP_ERROR NetworkProvisioning::SendCurrentIPv4Address()
{
for (chip::Inet::InterfaceAddressIterator it; it.HasCurrent(); it.Next())
Copy link
Contributor

Choose a reason for hiding this comment

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

At some point we probably want to propagate the source interface as well.

That seems really important for link-local and I imagine there could be unusual network setups where different network cards exist on different LANs.

Probably a TODO for the future.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@andy31415, this code NetworkProvisioning.h/.cpp will be deleted once we move the provisioning to ZCL cluster and DNS-SD. I'll file a GH issue in regards to that.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Filed #4606

@pan-apple
Copy link
Contributor Author

@saurabhst, any feedback?

@woody-apple
Copy link
Contributor

@saurabhst @jelderton ?

@woody-apple woody-apple merged commit 79964fc into project-chip:master Feb 2, 2021
@pan-apple pan-apple deleted the send-current-ip-np branch February 2, 2021 21:09
austinh0 pushed a commit to austinh0/connectedhomeip that referenced this pull request Feb 3, 2021
@pan-apple @austinh0
Send current IP address during network provisioning (project-chip#4591)
5d474bb
austinh0 pushed a commit to austinh0/connectedhomeip that referenced this pull request Feb 3, 2021
@pan-apple @austinh0
Send current IP address during network provisioning (project-chip#4591)
4bd39eb
austinh0 pushed a commit to austinh0/connectedhomeip that referenced this pull request Feb 4, 2021
@pan-apple @austinh0
Send current IP address during network provisioning (project-chip#4591)
b691496
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@andy31415 andy31415 andy31415 approved these changes

@saurabhst saurabhst saurabhst approved these changes

@woody-apple woody-apple woody-apple approved these changes

@BroderickCarlin BroderickCarlin Awaiting requested review from BroderickCarlin

@bzbarsky-apple bzbarsky-apple Awaiting requested review from bzbarsky-apple

@chrisdecenzo chrisdecenzo Awaiting requested review from chrisdecenzo

@hawk248 hawk248 Awaiting requested review from hawk248

@jelderton jelderton Awaiting requested review from jelderton

@mspang mspang Awaiting requested review from mspang

Assignees

No one assigned

Labels

transport

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@pan-apple @woody-apple @andy31415 @saurabhst