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

[nrf noup] Enhance/Optimize attach to thread network #324

Merged

Conversation

doublemis1
Copy link
Contributor

Enhance attaching to Thread by verifying if the current dataset is the same as provided.
In current solution implementation will not reset the Thread interface while the
dataset are equal to the active dataset.

Copy link
Contributor

@Damian-Nordic Damian-Nordic left a comment

Choose a reason for hiding this comment

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

A small suggestion to reduce the impact of the change.

Thread::OperationalDataset current_dataset;
// Validation the dataset change with the current state
ThreadStackMgrImpl().GetThreadProvision(current_dataset);
if (!dataset.AsByteSpan().data_equal(current_dataset.AsByteSpan()))
Copy link
Contributor

Choose a reason for hiding this comment

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

Hmm, maybe you could just do sth like:

if (dataset.AsByteSpan().data_equal(current_dataset.AsByteSpan() && callback == nullptr)
{
   return CHIP_NO_ERROR;
}
  1. If the callback is provided, we probably want to restart the interface. Otherwise, the callback will never be called, so if a user called ConnectNetwork for the second time, it would fail (I don't know why a user would want to do it, but it should probably work).
  2. If the callback is not provided we don't need to execute if (dataset.IsCommissioned()) code.

@doublemis1 doublemis1 force-pushed the ignore_reset_thread_interface branch from 658585d to c9d3034 Compare September 8, 2023 14:35
@doublemis1
Copy link
Contributor Author

Tested after update on NCS 2.2.0 Door Lock NRF52840:
How we can provide fix to clients (e.g. NCS 2.2.0)?

@kkasperczyk-no
Copy link
Contributor

Tested after update on NCS 2.2.0 Door Lock NRF52840: How we can provide fix to clients (e.g. NCS 2.2.0)?

Would creating a known issue with a recommendation to cherry-pick this commit work? Btw. could you open a PR to sdk-nrf with manifest change pointing to this PR? We need to test it against NCS main to be able to merge it.

Copy link
Contributor

@markaj-nordic markaj-nordic left a comment

Choose a reason for hiding this comment

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

@doublemis1 Please open the sdk-nrf PR with bumped manifest.

Enhance attaching to Thread by verifying if the
current dataset is the same as provided.
In current solution implementation will not
reset the Thread interface while the dataset
are equal to the active dataset.

Signed-off-by: Michał Szablowski <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants