Skip to content

Implement NFC commisisoning unpowered mode - #763

Open
ArekBalysNordic wants to merge 2 commits into
nrfconnect:masterfrom
ArekBalysNordic:st_commissioning
Open

ArekBalysNordic wants to merge 2 commits into
nrfconnect:masterfrom
ArekBalysNordic:st_commissioning

Conversation

@ArekBalysNordic

Copy link
Copy Markdown
Contributor

Summary

[nrf toup][nrfconnect] Adapt NFC Commissioning manager to unpowered mode

- Added define switch for powered and unpowered NFC Commissioning
modes
- Added Kconfigs to switch between these two modes.

[nrf toup][nrfconnect] Add NFC unpowered mode

- Added required functions to allow performing NFC commissioning
- Added an event to handle connecting to operational network
- Added Opening commissioning window for NFC purposes
- Updated NetworkCommissioningCluster to use the new event

Testing

ncs-matter

manifest-pr-skip

}
}

void CommissioningWindowManager::OnNfcBasedCommissioningStarting()

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.

Shouldn't this code be under CHIP_NFC_BASED_COMMISSIONING ifdef?


/**
* Signals to connect the device to the operational network using credentials
* received from unpowered NFC commissioning data.

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.

Is NFC the only use case for that signal? If yes, shouldn't handling be guarded by ifdef (and named in less generic way)?

debugText = MutableCharSpan(debugTextBuffer);
#endif
uint8_t outNetworkIndex = 0;
NetworkCommissioningStatusEnum networkingStatus = this_->mpDriver.Get<ThreadDriver *>()->AddOrUpdateNetwork(

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 it would need some ifdef as well, because calling ThreadDriver directly will not compile for Wi-Fi or Ethernet.

ISO-DEP (Type 4A) session.

config CHIP_NFC_COMMISSIONING_MODE_UNPOWERED
bool "Unpowered (external ST25DA-C via I2C)"

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 would not mention ST and I2C here. Let's keep it generic - companion/external NFC tag

prompt "NFC commissioning mode"
default CHIP_NFC_COMMISSIONING_MODE_POWERED

config CHIP_NFC_COMMISSIONING_MODE_POWERED

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.

This is more like a question than request. I'm a bit concerned about our naming. It follows the requests that we got, but in fact when choosing "unpowered" mode it could be that we will handle it in "powered" way, right? It's more like "powered" means "powered only" and "unpowered" means "unpowered or powered". Also I'm not sure that it explains well why different handling in software is needed.

I was thinking about "on-chip" and "external chip" but it is true for our SoC, some other vendors may support unpowered mode with their MCU. Maybe this would be better to name it like single step or two step mode? I don't know...

- Added required functions to allow performing NFC commissioning
- Added an event to handle connecting to operational network
- Added Opening commissioning window for NFC purposes
- Updated NetworkCommissioningCluster to use the new event
- Added define switch for powered and unpowered NFC Commissioning
modes
- Added Kconfigs to switch between these two modes.

Signed-off-by: Arkadiusz Balys <arkadiusz.balys@nordicsemi.no>
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.

2 participants