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

tests/pkg_tinyvcdiff: Initialize mtd->write_size #18529

Merged
merged 1 commit into from
Aug 30, 2022

Conversation

MrKevinWeiss
Copy link
Contributor

Contribution description

An assert assert(mtd->write_size != 0); is being hit with native.

main(): This is RIOT! (Version: buildtest)
..drivers/mtd/mtd.c:53 => *** RIOT kernel panic:
FAILED ASSERTION.

*** halted.

It seems that initializing this to 1 in the fakemtd fixes it.
Not that I know anything about what it should be. If one looks at some of the other write_sizes it seems that would make sense.

Testing procedure

Run the following, it should not longer fail:

make all test -C tests/pkg_tinyvcdiff/

I think murdock should have caught it... Maybe add CI: run_tests label.

Issues/PRs references

Introduced with the initial test #17797

An assert `assert(mtd->write_size != 0);` is being hit.
It seems that initializing this to 1 in the fakemtd fixes it.
Not that I know anything about what it should be.
@github-actions github-actions bot added the Area: tests Area: tests and testing framework label Aug 30, 2022
@MrKevinWeiss MrKevinWeiss added the CI: run tests If set, CI server will run tests on hardware for the labeled PR label Aug 30, 2022
@MrKevinWeiss
Copy link
Contributor Author

ping @jue89 if this makes sense?

@@ -65,7 +65,8 @@ typedef struct {
#define FAKE_MTD_INIT { .mtd = { .driver = &fake_mtd_driver, \
.sector_count = FAKE_MTD_SECTOR_COUNT, \
.pages_per_sector = FAKE_MTD_PAGES_PER_SECTOR, \
.page_size = FAKE_MTD_PAGE_SIZE } }
.page_size = FAKE_MTD_PAGE_SIZE, \
.write_size = 1 } }
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe setting this to 4 (FAKE_MTD_WRITE_SIZE) or would be more interesting

Copy link
Member

Choose a reason for hiding this comment

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

As the write granularity is not enforced by the fake_mtd_driver, I wouldn't expect anything interesting.

Copy link
Contributor

@jue89 jue89 left a comment

Choose a reason for hiding this comment

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

Yes, seems right! write_size indicates alignment and minimum amount of bytes to write on the underlying mtd. The fakemtd can be written byte-wise.

@chrysn chrysn added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Aug 30, 2022
@benpicco benpicco merged commit 034e395 into RIOT-OS:master Aug 30, 2022
@MrKevinWeiss MrKevinWeiss deleted the pr/fix/pkg_tinyvcdiff branch August 30, 2022 14:15
@MrKevinWeiss
Copy link
Contributor Author

MrKevinWeiss commented Aug 30, 2022

Thanks for the quick responses!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: tests Area: tests and testing framework CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR CI: run tests If set, CI server will run tests on hardware for the labeled PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants