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

[feature-wisun] Improve WisunInterface set_network_size docs #14431

Merged

Conversation

artokin
Copy link
Contributor

@artokin artokin commented Mar 15, 2021

Summary of changes

Clarify set_network_size usage and possible parameter values.

Impact of changes

Migration actions required

Documentation


Pull request type

[x] Patch update (Bug fix / Target update / Docs update / Test update / Refactor)
[] Feature update (New feature / Functionality change / New API)
[] Major update (Breaking change E.g. Return code change / API behaviour change)

Test results

[x] No Tests required for this change (E.g docs only update)
[] Covered by existing mbed-os tests (Greentea or Unittest)
[] Tests / results supplied as part of this PR

Reviewers


@artokin artokin requested a review from mikter March 15, 2021 14:23
@ciarmcom ciarmcom requested a review from a team March 15, 2021 14:30
@ciarmcom
Copy link
Member

@artokin, thank you for your changes.
@ARMmbed/mbed-os-maintainers please review.

0xc0170
0xc0170 previously approved these changes Mar 15, 2021
@mbed-ci
Copy link

mbed-ci commented Mar 16, 2021

Test run: SUCCESS

Summary: 10 of 10 test jobs passed
Build number : 1
Build artifacts

*
* The network size is measured as hundreds of devices. For example:
* - set network size to 1 to use Wi-SUN network with less than 100 devices (small).
* - set network size to 8 to use Wi-SUN network with 100-800 devices (medium).
Copy link

Choose a reason for hiding this comment

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

We need to remove the limits on devices as the amount of devices is dependent on the data rate so
medium settings is used from 500, 1000, 1500 to 2500 devices depending the data rate

So it would be better to just say that there is 4 buckets of settings which are selected based on data rate and device amount

This field ONLY gives the device amount
so
1 = 100
2=200
5=500
20=2000

@artokin artokin force-pushed the update_set_network_size_documentation branch from c5fee38 to 4b96ef6 Compare March 17, 2021 09:54
@mergify mergify bot dismissed 0xc0170’s stale review March 17, 2021 09:54

Pull request has been modified.

@artokin
Copy link
Contributor Author

artokin commented Mar 17, 2021

Rebased and updated documentation based on the review comments. Would you please review again?

@artokin artokin requested a review from mikter March 17, 2021 11:25
@0xc0170
Copy link
Contributor

0xc0170 commented Mar 17, 2021

CI started

@@ -121,7 +121,7 @@
"value": "\"Wi-SUN Network\""
},
"wisun-network-size": {
"help": "Expected amount of devices in the network as 100s of devices. with possible pre defined constants NETWORK_SIZE_SMALL, NETWORK_SIZE_MEDIUM, NETWORK_SIZE_LARGE, NETWORK_SIZE_XLARGE. if set to 0 Wi-SUN Certification configuration values are used. If don't define this(default null), then NETWORK_SIZE_MEDIUM will be used.",
"help": "Expected amount of devices in the network as hundreds of devices. Use any number or one of the predefined values NETWORK_SIZE_SMALL, NETWORK_SIZE_MEDIUM, NETWORK_SIZE_LARGE or NETWORK_SIZE_XLARGE. Set to 0 to use Wi-SUN Certification configuration values. If set to null, then NETWORK_SIZE_MEDIUM (meaning up to 800 devices in 150kbs data rate) will be used.",
Copy link

Choose a reason for hiding this comment

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

meaning up to 800 devices in 150kbs data rate -> Meaning hundreds of devices in the network

80 is going to be changed but it always is hundreds

@mbed-ci
Copy link

mbed-ci commented Mar 17, 2021

Test run: SUCCESS

Summary: 10 of 10 test jobs passed
Build number : 2
Build artifacts

@artokin artokin force-pushed the update_set_network_size_documentation branch 2 times, most recently from 0326880 to 0cce166 Compare March 17, 2021 13:19
mikter
mikter previously approved these changes Mar 17, 2021
@mergify mergify bot added needs: CI and removed needs: review labels Mar 17, 2021
Clarify set_network_size usage and possible parameter values.
Clarify wisun-network-size help text in mbed-mesh-api/mbed_lib.json
@artokin artokin force-pushed the update_set_network_size_documentation branch from 0cce166 to 77b9eb5 Compare March 18, 2021 09:45
@mergify mergify bot dismissed mikter’s stale review March 18, 2021 09:45

Pull request has been modified.

@artokin artokin requested a review from tz-arm March 18, 2021 09:46
@artokin artokin changed the title Improve WisunInterface set_network_size docs [feature-wisun] Improve WisunInterface set_network_size docs Mar 18, 2021
* - Timing settings set by set_timing_parameters() of the Wi-SUN interface.
* - RPL settings set by rpl_parameters_set() of the Border Router interface.
*
* When network size is changed, and if timing or RPL values should be other than defaults set by stack for the network size,
* they need to set again using above function calls.
* When the network size is changed, and if timing or RPL values should be other than defaults set by stack for the
Copy link
Contributor

Choose a reason for hiding this comment

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

This is only change Border router RPL configs.

Copy link
Contributor

@tz-arm tz-arm Mar 22, 2021

Choose a reason for hiding this comment

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

There will be "configuration buckets based on the selected network size and data rate" for the four options, but is there also suitable configuration buckets for other values like"4"? or application need to handle other configurations by itself?
If no configuration buckets for other size, there are two API list here, set_timing_parameters/rpl_parameters_set, is it meant once change network to "4", only there two parameters need to change? how?

Copy link

Choose a reason for hiding this comment

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

Only configuration Application needs to set is the network size and the stack will select the correct configuration that it will use based on the network size and data rate

All values from 1 to 254 will result in some selection made by stack so any value can be given even 4 meaning expected amount of devices in network is 400 devices.

* - 1 to use small configuration bucket.
* - 8 to use medium configuration bucket.
* - 15 to use large configuration bucket.
* - 25 to use xlarge configuration bucket.
Copy link
Contributor

Choose a reason for hiding this comment

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

If we prefer customer to use the "number of hundreds devices" as network size, I would propose to remove all concept, micros and comments about " small, medium, large, xlarge", the duplicate concept/usage will cause confusion。

*
* When network size is changed, it will override all or some of the following configuration values:
* When the network size is changed, it will override all or some of the following configuration values:
Copy link
Contributor

Choose a reason for hiding this comment

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

Propose to use ” the network size setting will refresh and generate new configurations bucket based on bandwidth the input network size value.
If the customized timing or RPL values is needed, the below two API should be invoked after network size setting.

Updated based on review comments.
@artokin artokin requested a review from tz-arm March 23, 2021 09:46
* When network size is changed, it will override all or some of the following configuration values:
* - Timing settings set by set_timing_parameters() of the Wi-SUN interface.
* - RPL settings set by rpl_parameters_set() of the Border Router interface.
* By default the Wi-SUN stack is configured to use a few hundreds of devices.
Copy link
Contributor

Choose a reason for hiding this comment

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

That's seems simple and clear, thanks for the update!
Just one question, if the "by default ... a few hundreds" should be "4 means 4 hundreds" as default?

@adbridge
Copy link
Contributor

Ci started

@mbed-ci
Copy link

mbed-ci commented Mar 25, 2021

Test run: SUCCESS

Summary: 10 of 10 test jobs passed
Build number : 3
Build artifacts

@adbridge adbridge merged commit beebd93 into ARMmbed:feature-wisun Mar 26, 2021
@mergify mergify bot removed the ready for merge label Mar 26, 2021
@artokin artokin deleted the update_set_network_size_documentation branch March 26, 2021 15:04
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

8 participants