-
Notifications
You must be signed in to change notification settings - Fork 3k
Make devicekey remainder test more meaningful #11628
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
Make devicekey remainder test more meaningful #11628
Conversation
As of 722628b, the "remainder" configuration also uses the default location near the end of flash. Which makes the two tests nearly identical with the exception that the "last two sectors" test correctly handles parts with a low (possibly 1:1) erase size to program size ratio. Therefore, change the "remainder" test to instead be a "default" test that uses the tdb_internal_address/size values, so that it a.) tests something meaningfully different and b.) tests using the custom TDB address/size values if they are provided. c.) functions correctly on devices where the default sector-based size computation does not work (e.g. because of the low erase size to program size ratio) and therefore a custom location and size has been specified. The is_conf_tdb_internal variable is unused and therefore removed.
@kyle-cypress, thank you for your changes. |
@SeppoTakalo please review |
@@ -50,7 +50,7 @@ static inline uint32_t align_down(uint64_t val, uint64_t size) | |||
return (((val) / size)) * size; | |||
} | |||
|
|||
int get_virtual_TDBStore_position(uint32_t conf_start_address, uint32_t conf_size, bool is_conf_tdb_internal, | |||
int get_virtual_TDBStore_position(uint32_t conf_start_address, uint32_t conf_size, bool is_conf_tdbd_internal, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The parameter doesn't seem to be used for anything actually.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed - I think this is a vestige from the old version of the test prior to 722628b . I'll push another commit to remove.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removed the unused parameter
e4cd4a0
to
1cb6f7b
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me.
I restarted Travis and started CI |
Test run: SUCCESSSummary: 5 of 5 test jobs passed |
Description
As of 722628b, the "remainder" configuration also uses the default location near the end of flash. Which makes the two tests nearly identical with the exception that the "last two sectors" test correctly handles parts with a low (possibly 1:1) erase size to program size ratio.
Therefore, change the "remainder" test to instead be a "default" test that uses the tdb_internal_address/size values, so that it
a.) tests something meaningfully different
b.) tests using the custom TDB address/size values if they are provided.
c.) functions correctly on devices where the default sector-based size computation does not work (e.g. because of the low erase size to program size ratio) and therefore a custom location and size has been specified.
The is_conf_tdb_internal variable is unused and therefore removed.
Pull request type
Reviewers
Release Notes