Skip to content

Simplify API by removing RNG interface #8

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

Merged
merged 8 commits into from
Nov 25, 2020

Conversation

aggarw13
Copy link
Contributor

@aggarw13 aggarw13 commented Nov 25, 2020

To improve usability of API, the existing BackoffAlgorithm_RNG_t interface for passing a random number generator function to the library is removed in this PR. This helps avoid the need of "wrappers" around platform-specific RNGs to pass to the library. Instead, the application can generate the random number using their platform-specific RNG and just directly pass to the library for calculating the next retry backoff period.

Note: Removal of the random number generator dependency makes this library completely platform-independent!

leegeth
leegeth previously approved these changes Nov 25, 2020
@cobusve cobusve changed the title Simplify API be removing RNG interface Simplify API by removing RNG interface Nov 25, 2020
*
* For the simplicity of the code example, the pseudo random number generator, rand() function
* is used. */
retryStatus = BackoffAlgorithm_GetNextBackoff( &retryParams, rand(), &nextRetryBackOff );
Copy link
Member

Choose a reason for hiding this comment

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

Where is this PRNG seeded? I ask because if you do an OTA on a fleet they will all start up with the same seed and you will have exactly the connection storm this is intended to avoid.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good catch. I have added seeding logic of RNG in the example.

leegeth
leegeth previously approved these changes Nov 25, 2020
Comment on lines 121 to 122
* the random value. Using a random number generator that generates device-specific unique values
* specific mitigates possibility of collision between multiple devices retrying the network operations.

Choose a reason for hiding this comment

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

This last sentence seems problematic to me. A TRNG does not generate unique values, it generates random values. The entropy pool may be device specific, but conceptually the random number is not device-specific; it's just random.

Copy link
Member

Choose a reason for hiding this comment

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

Yes I agree we should just say that using random adds diversity, too much detail here is just a trap, random is hard.

cobusve
cobusve previously approved these changes Nov 25, 2020
@aggarw13 aggarw13 merged commit 8a9a665 into FreeRTOS:main Nov 25, 2020
@aggarw13 aggarw13 deleted the refactor/simplify-api branch November 25, 2020 21:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants