RIOT
RIOT copied to clipboard
RIOT-OS
cpu/native: introduce periph_i2c_mock
Contribution description
This allows I2C emulation on native architecture in the same way than periph_gpio_mock.
All I2C function from this driver are set as weak to be easily overridden in each application.
Testing procedure
$ make -C tests/periph/i2c/ BOARD=native
make : on entre dans le répertoire « /home/gdo/Developpement/Informatique/Personnel/cogip/RIOT/tests/periph/i2c »
Building application "tests_i2c" for "native" with MCU "native".
"make" -C /home/gdo/Developpement/Informatique/Personnel/cogip/RIOT/boards/common/init
"make" -C /home/gdo/Developpement/Informatique/Personnel/cogip/RIOT/boards/native
"make" -C /home/gdo/Developpement/Informatique/Personnel/cogip/RIOT/boards/native/drivers
"make" -C /home/gdo/Developpement/Informatique/Personnel/cogip/RIOT/core
"make" -C /home/gdo/Developpement/Informatique/Personnel/cogip/RIOT/core/lib
"make" -C /home/gdo/Developpement/Informatique/Personnel/cogip/RIOT/cpu/native
"make" -C /home/gdo/Developpement/Informatique/Personnel/cogip/RIOT/cpu/native/periph
"make" -C /home/gdo/Developpement/Informatique/Personnel/cogip/RIOT/cpu/native/stdio_native
"make" -C /home/gdo/Developpement/Informatique/Personnel/cogip/RIOT/drivers
"make" -C /home/gdo/Developpement/Informatique/Personnel/cogip/RIOT/drivers/periph_common
"make" -C /home/gdo/Developpement/Informatique/Personnel/cogip/RIOT/sys
"make" -C /home/gdo/Developpement/Informatique/Personnel/cogip/RIOT/sys/auto_init
"make" -C /home/gdo/Developpement/Informatique/Personnel/cogip/RIOT/sys/div
"make" -C /home/gdo/Developpement/Informatique/Personnel/cogip/RIOT/sys/frac
"make" -C /home/gdo/Developpement/Informatique/Personnel/cogip/RIOT/sys/libc
"make" -C /home/gdo/Developpement/Informatique/Personnel/cogip/RIOT/sys/preprocessor
"make" -C /home/gdo/Developpement/Informatique/Personnel/cogip/RIOT/sys/shell
"make" -C /home/gdo/Developpement/Informatique/Personnel/cogip/RIOT/sys/shell/cmds
"make" -C /home/gdo/Developpement/Informatique/Personnel/cogip/RIOT/sys/ztimer
text data bss dec hex filename
45915 844 47996 94755 17223 /home/gdo/Developpement/Informatique/Personnel/cogip/RIOT/tests/periph/i2c/bin/native/tests_i2c.elf
make : on quitte le répertoire « /home/gdo/Developpement/Informatique/Personnel/cogip/RIOT/tests/periph/i2c »
Murdock results
:x: FAILED
03fc4167f619fc037a2f8fc5e08821922dbb80cf tests/drivers/pca9685: fix printf format
| Success | Failures | Total | Runtime |
|---|---|---|---|
| 2112 | 0 | 9595 | 03m:24s |
Artifacts
Murdock results
❌ FAILED
e5e81d1 drivers/bme680: fix empty initializer
Success Failures Total Runtime 97 0 9367 01m:27s
Artifacts
@chrysn thanks for your suggestions, applied them. I added a commit to try to fix murdock test.
There's still a build error left, and I'd appreciated if the mocking part was somehow shown / tested.
I've done both, but can't push directly to this branch, but you can pick the commits from https://github.com/cogip/RIOT/pull/1.
There's still a build error left, and I'd appreciated if the mocking part was somehow shown / tested.
I've done both, but can't push directly to this branch, but you can pick the commits from cogip#1.
Thx for patches @chrysn . Applied and tested:
> i2c_write_bytes 0 10 0 0 1 0 1 2
i2c_write_bytes 0 10 0 0 1 0 1 2
Command: i2c_write_bytes(0, 0x0a, 0x00, [0x00, 0x01, 0x00, 0x01, 0x02])
Mock write intercepted; this write of 5 byte(s) is still ignored.
Success: i2c_0 wrote 5 bytes
Great, let's go with this.
The failing test appears to expose an issue with the si1133 driver (missing includes), can you fix that in passing?
done :crossed_fingers:
Seems you've found a particular whack-a-mole of missing includes for macros (now drivers/include/hm330x.h relies on someone else to import its IS_USED); could you fix one more?
Seems you've found a particular whack-a-mole of missing includes for macros (now
drivers/include/hm330x.hrelies on someone else to import its IS_USED); could you fix one more?
yeah dont worry, I'll fix them all, I tell you once done for fix reviews. ;)
Maybe something along the lines of this fixes the most recent build failure?
diff --git a/pkg/cryptoauthlib/Makefile b/pkg/cryptoauthlib/Makefile
index 555e4ce1bf..56b7b7a309 100644
--- a/pkg/cryptoauthlib/Makefile
+++ b/pkg/cryptoauthlib/Makefile
@@ -40,6 +40,7 @@ $(PKG_BUILD_DIR)/Makefile: $(TOOLCHAIN_FILE)
cmake -DCMAKE_TOOLCHAIN_FILE=$(TOOLCHAIN_FILE) \
-Wno-dev \
-DATCA_NO_HEAP:BOOL=TRUE \
+ -DCMAKE_C_STANDARD=c11 \
-B$(PKG_BUILD_DIR) \
-H$(PKG_SOURCE_DIR)/lib
@maribu thx, good start but it cannot be changed by command line and it was the ball of whool :sweat_smile: . Fixed it, it builds in native(64) and original board. I hope there will be no regression.
@chrysn more than 30 fixes to pass murdock test. More than expected :sweat_smile: .
My concerns about these fixes:
- size_t real size vary according to architecture, so when size_t was used, I cast to uint32_t. Seems not the best but as it is mainly for display in printf, it could be acceptable, I let you check.
- cryptoauthlib seems to be an important package, even if my patches and modifications are minor, that could be good to test all is ok with someone who knows well that part.
- Do you think it would be proper to put these fixes in https://github.com/RIOT-OS/RIOT/pull/20431 ?
On size_t printing, there is "%zu"; did you try whether that works in our case?
On the place for the fixes, I think these are the right place -- it is periph_i2c_mock that makes those modules buildable (or at least built-in-CI) on native, and therefore the fixes fit.
The cryptoauthlib changes seem innocent enough; in particular, the places where they are not trivial in the sense that they're good if they compile are only in tests.
On size_t printing, there is "%zu"; did you try whether that works in our case?
Seems the good way, indeed. :wink:
All seems ok now.
%zuia not supported by newlib, sadly ...
Indeed, --enable-newlib-io-c99-formats is not set by default. :thinking: Weird it builds however...
What should we do around this ?
What should we do around this ?
The current work around is to use printf("Size of foobar is %" PRIuSIZE " B\n", (size_t)foobar_size);. The define is provided by #include "architecture.h".
See https://api.riot-os.org/group__sys__architecture.html#ga2b7b0557dc6cd786df02dafbb51f5292
Maybe we will eventually just ditch newlib or at least newlib's stdio. It is surprising how they manage to provide both the most bloated and least feature complete stdio implementation at the same time.
Thx @chrysn @maribu . I think we are good now.
I tested with native, native64 and on nucleo-g431rb with this small diff:
diff --git a/examples/hello-world/main.c b/examples/hello-world/main.c
index 213128ac64..e25fb9ee74 100644
--- a/examples/hello-world/main.c
+++ b/examples/hello-world/main.c
@@ -19,6 +19,7 @@
* @}
*/
+#include <architecture.h>
#include <stdio.h>
int main(void)
@@ -27,6 +28,7 @@ int main(void)
printf("You are running RIOT on a(n) %s board.\n", RIOT_BOARD);
printf("This board features a(n) %s CPU.\n", RIOT_CPU);
+ printf("Dummy size_t test %"PRIuSIZE"\n", (size_t)SIZE_MAX);
return 0;
}
On nucleo-g431rb:
main(): This is RIOT! (Version: 2024.04-devel-387-g6a16b7-native_i2c_mock)
Hello World!
You are running RIOT on a(n) nucleo-g431rb board.
This board features a(n) stm32 CPU.
Dummy size_t test 4294967295
On native:
main(): This is RIOT! (Version: 2024.04-devel-387-g6a16b7-native_i2c_mock)
Hello World!
You are running RIOT on a(n) native board.
This board features a(n) native CPU.
Dummy size_t test 4294967295
On native64:
main(): This is RIOT! (Version: 2024.04-devel-387-g6a16b7-native_i2c_mock)
Hello World!
You are running RIOT on a(n) native64 board.
This board features a(n) native CPU.
Dummy size_t test 18446744073709551615
Update: Feel free to squash at will.
Thx for review. I'm not a fan of big squash but I could squash per file it is ok for you @maribu ?
Update: Feel free to squash at will.
Thx for review. I'm not a fan of big squash but I could squash per file it is ok for you @maribu ?
Ideally the resulting commits should all contain a collection of strongly related changes with a single intent. There is a lot of interpretation room and it is pretty subjective how fine-grained commits should be.
I personally try to look at this from the "revert" perspective: In case a regression sneeks in that cannot easily be fixed, what would be sensible units for reverting?
Note: At the current ponty in the release cycle this will likely not get merged, but has to wait for the release branch to fork of. That should be done soonish. Afterwards this can be merged.