[riot-notifications] [RIOT-OS/RIOT] drivers/mrd_spi_nor: fix read spanning pages (#11081)

Ga√ętan Harter notifications at github.com
Thu Feb 28 16:40:24 CET 2019


I think it is important to mention it must be tested on a board using `mtd_spi_nor` and so only on `mulle`.
```
BOARD=mulle make -C tests/unittests/ tests-mtd flash test
```
Otherwise it does not test anything and the test pass without the fix in the drivers.

With the fix the test work on `mulle`:


```
BOARD=mulle make -C tests/unittests/ tests-mtd flash test

2019-02-28 16:27:02,401 - INFO # main(): This is RIOT! (Version: 2019.04-devel-309-gfd42af-pr/riot/11081/fix_read_spanning_pages)
2019-02-28 16:27:02,481 - INFO # main(): This is RIOT! (Version: 2019.04-devel-309-gfd42af-pr/riot/11081/fix_read_spanning_pages)
2019-02-28 16:27:15,648 - INFO # ......
2019-02-28 16:27:15,649 - INFO # OK (6 tests)
``````

Without the fix in the driver the test fail:

```
2019-02-28 16:20:55,334 - INFO # main(): This is RIOT! (Version: 2019.04-devel-309-gfd42af-pr/riot/11081/fix_read_spanning_pages)
2019-02-28 16:20:55,399 - INFO # main(): This is RIOT! (Version: 2019.04-devel-309-gfd42af-pr/riot/11081/fix_read_spanning_pages)
2019-02-28 16:21:04,662 - INFO # ....
2019-02-28 16:21:04,678 - INFO # mtd_tests.test_mtd_write_read (tests/unittests/tests-mtd/tests-mtd.c 232) exp 257 was 256
2019-02-28 16:21:08,582 - INFO # ..
2019-02-28 16:21:08,583 - INFO # run 6 failures 1
```                                                                                                                                                                                          

However, only these two lines in the test failed with the new code:

``` diff
diff --git a/tests/unittests/tests-mtd/tests-mtd.c b/tests/unittests/tests-mtd/tests-mtd.c
index 17cbaf28f..03e2823c3 100644
--- a/tests/unittests/tests-mtd/tests-mtd.c
+++ b/tests/unittests/tests-mtd/tests-mtd.c
@@ -229,9 +229,9 @@ static void test_mtd_write_read(void)
     TEST_ASSERT_EQUAL_INT(dev->page_size, ret);
     memset(buf_page, 0, sizeof(buf_page));
     ret = mtd_read(dev, buf_page, 0, sizeof(buf_page));
-    TEST_ASSERT_EQUAL_INT(sizeof(buf_page), ret);
+    //TEST_ASSERT_EQUAL_INT(sizeof(buf_page), ret);
     TEST_ASSERT_EQUAL_INT(1, buf_page[0]);
-    TEST_ASSERT_EQUAL_INT(1, buf_page[sizeof(buf_page) - 1]);
+    //TEST_ASSERT_EQUAL_INT(1, buf_page[sizeof(buf_page) - 1]);

     /* pages overlap write */
     ret = mtd_write(dev, buf, dev->page_size - (sizeof(buf) / 2), sizeof(buf));
```


Is that intentional ? If it is, for me it would make sense to split the commit in different steps.

* add the normal sanity checks that were already working in a commit
* Then each fix in driver with its corresponding `assert` in the test


-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/RIOT-OS/RIOT/pull/11081#issuecomment-468320088
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.riot-os.org/pipermail/notifications/attachments/20190228/02cf9434/attachment.html>


More information about the notifications mailing list