[go: nahoru, domu]

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

[dataset-manager] apply partial dataset config on channel set failure #10490

Merged
merged 1 commit into from
Jul 10, 2024

Conversation

abtink
Copy link
Member
@abtink abtink commented Jul 8, 2024

This commit updates DatasetManager::ApplyConfiguration() to apply the rest of the parameters in the Dataset even if setting the channel fails.

@abtink abtink requested a review from EskoDijk July 8, 2024 20:54
@abtink abtink marked this pull request as ready for review July 8, 2024 20:54
Copy link
size-report bot commented Jul 8, 2024

Size Report of OpenThread

Merging #10490 into main(bf5ddb9).

name branch text data bss total
ot-cli-ftd main 466656 856 66364 533876
#10490 466656 856 66364 533876
+/- 0 0 0 0
ot-ncp-ftd main 435860 760 61592 498212
#10490 435860 760 61592 498212
+/- 0 0 0 0
libopenthread-ftd.a main 236010 95 40302 276407
#10490 236010 95 40302 276407
+/- 0 0 0 0
libopenthread-cli-ftd.a main 57524 0 8075 65599
#10490 57524 0 8075 65599
+/- 0 0 0 0
libopenthread-ncp-ftd.a main 32127 0 5924 38051
#10490 32127 0 5924 38051
+/- 0 0 0 0
ot-cli-mtd main 365264 760 51236 417260
#10490 365264 760 51236 417260
+/- 0 0 0 0
ot-ncp-mtd main 348092 760 46480 395332
#10490 348092 760 46480 395332
+/- 0 0 0 0
libopenthread-mtd.a main 158715 0 25190 183905
#10490 158719 0 25190 183909
+/- +4 0 0 +4
libopenthread-cli-mtd.a main 39739 0 8059 47798
#10490 39739 0 8059 47798
+/- 0 0 0 0
libopenthread-ncp-mtd.a main 25007 0 5924 30931
#10490 25007 0 5924 30931
+/- 0 0 0 0
ot-cli-ftd-br main 550584 864 131228 682676
#10490 550584 864 131228 682676
+/- 0 0 0 0
libopenthread-ftd-br.a main 324295 100 105142 429537
#10490 324295 100 105142 429537
+/- 0 0 0 0
libopenthread-cli-ftd-br.a main 71437 0 8099 79536
#10490 71437 0 8099 79536
+/- 0 0 0 0
ot-rcp main 62312 564 20636 83512
#10490 62312 564 20636 83512
+/- 0 0 0 0
libopenthread-rcp.a main 9734 0 5060 14794
#10490 9734 0 5060 14794
+/- 0 0 0 0
libopenthread-radio.a main 18991 0 222 19213
#10490 18991 0 222 19213
+/- 0 0 0 0

@@ -199,8 +199,7 @@ Error DatasetManager::ApplyConfiguration(const Dataset &aDataset) const

if (error != kErrorNone)
{
LogWarn("ApplyConfiguration() Failed to set channel to %d (%s)", channel, ErrorToString(error));
ExitNow();
LogWarn("Failed to set channel to %u when applying dataset: %s", channel, ErrorToString(error));
Copy link
Contributor

Choose a reason for hiding this comment

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

What about LogCrit() here? LogWarn is normally pretty mild like e.g. failing to send a particular message (which can be easily retried) or not able to store some extra items. While this error is critical: the dataset defines a channel that the device can't support, and that's pretty final. (Typically hardcoded in the platform in the supported-channels mask.) So the entire Thread device can't operate anymore as part of the network, on the intended channel.

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks @EskoDijk . I think using LogCrit() can be useful. Updated in new push.

This commit updates `DatasetManager::ApplyConfiguration()` to apply
the rest of the parameters in the Dataset even if setting the channel
fails. The channel error is now logged as `LogCrit()`.
@abtink abtink force-pushed the dataset/apply-config-error branch from 295c9d0 to a2f70e5 Compare July 9, 2024 17:36
@jwhui jwhui requested a review from EskoDijk July 9, 2024 18:16
Copy link
Contributor
@EskoDijk EskoDijk left a comment

Choose a reason for hiding this comment

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

Ok, looks good now!

@jwhui jwhui merged commit 32fe4c7 into openthread:main Jul 10, 2024
103 checks passed
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.

None yet

3 participants