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

cpu/native: introduce periph_i2c_mock #20430

Open
wants to merge 30 commits into
base: master
Choose a base branch
from

Conversation

gdoffe
Copy link
Contributor

@gdoffe gdoffe commented Feb 24, 2024

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 »

@github-actions github-actions bot added Platform: native Platform: This PR/issue effects the native platform Area: build system Area: Build system Area: cpu Area: CPU/MCU ports labels Feb 24, 2024
@chrysn chrysn added Reviewed: 1-fundamentals The fundamentals of the PR were reviewed according to the maintainer guidelines Reviewed: 2-code-design The code design of the PR was reviewed according to the maintainer guidelines Reviewed: 4-code-style The adherence to coding conventions by the PR were reviewed according to the maintainer guidelines CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR labels Feb 25, 2024
@riot-ci
Copy link

riot-ci commented Feb 25, 2024

Murdock results

✔️ PASSED

2720856 tests/drivers/pca9685: fix printf format

Success Failures Total Runtime
10619 0 10619 09m:49s

Artifacts

@github-actions github-actions bot added the Area: drivers Area: Device drivers label Feb 26, 2024
@gdoffe
Copy link
Contributor Author

gdoffe commented Feb 26, 2024

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.

@chrysn
Copy link
Member

chrysn commented Feb 26, 2024

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.

@chrysn chrysn added Reviewed: 3-testing The PR was tested according to the maintainer guidelines Reviewed: 5-documentation The documentation details of the PR were reviewed according to the maintainer guidelines labels Feb 26, 2024
@github-actions github-actions bot added the Area: tests Area: tests and testing framework label Feb 28, 2024
@gdoffe gdoffe force-pushed the native_i2c_mock branch 2 times, most recently from fc45e56 to 333dc02 Compare February 28, 2024 15:23
@gdoffe
Copy link
Contributor Author

gdoffe commented Feb 28, 2024

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

Copy link
Member

@chrysn chrysn left a comment

Choose a reason for hiding this comment

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

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?

@gdoffe
Copy link
Contributor Author

gdoffe commented Feb 28, 2024

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 🤞

@chrysn
Copy link
Member

chrysn commented Feb 28, 2024

Awesome, thanks!

Fix:
  bme680.c:42:39: error: use of GNU empty initializer extension
  bme680_t *bme680_devs[BME680_NUMOF] = { };
                                      ^

Signed-off-by: Gilles DOFFE <[email protected]>
- <stdbool.h> is missing for bool type definition.
- "container.h" is missing for ARRAY_SIZE macro definition.

Signed-off-by: Gilles DOFFE <[email protected]>
Fix:
drivers/include/hm330x.h:66:12: error: missing binary operator before
token "("

Signed-off-by: Gilles DOFFE <[email protected]>
Fix:
drivers/lsm6dsxx/include/lsm6dsxx_internal.h:148:47: error: binary
constants are a C2X feature or GCC extension

Signed-off-by: Gilles DOFFE <[email protected]>
Fix:
drivers/include/mtd_at24cxxx.h:49:1: error: initializer element is not
constant [-Werror=pedantic]
   49 | (mtd_at24cxxx_t) {                                  \
      | ^

Signed-off-by: Gilles DOFFE <[email protected]>
Fix:
  error: format ‘%p’ expects argument of type ‘void *’

and:
  error: ISO C forbids empty initializer braces [-Werror=pedantic]

Signed-off-by: Gilles DOFFE <[email protected]>
The pcf857x_gpio_write() is also returning void as pcf857x_gpio_*
functions.

This fixes following errors using native architecture:
RIOT/drivers/pcf857x/pcf857x.c:319:12: error: ISO C forbids ‘return’
with expression, in function returning void [-Werror=pedantic]
  319 |     return pcf857x_gpio_write(dev, pin, 0);
      |            ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Signed-off-by: Gilles DOFFE <[email protected]>
Fix:
  error: format ‘%p’ expects argument of type ‘void *’

Signed-off-by: Gilles DOFFE <[email protected]>
Fix:
  error: ISO C forbids empty initializer braces [-Werror=pedantic]

and:
  error: ISO C requires a translation unit to contain at least one
  declaration [-Werror,-Wempty-translation-unit]

Signed-off-by: Gilles DOFFE <[email protected]>
Fix:
  error: ISO C forbids empty initializer braces [-Werror=pedantic]

Signed-off-by: Gilles DOFFE <[email protected]>
Fix:
  error: overflow in conversion from ‘int’ to ‘char’ changes value from
  ‘255’ to ‘-1’ [-Werror=overflow]

Signed-off-by: Gilles DOFFE <[email protected]>
Fix:
  error: ISO C forbids empty initializer braces [-Werror=pedantic]

Signed-off-by: Gilles DOFFE <[email protected]>
Fix:
  error: binary constants are a C2X feature or GCC extension

Signed-off-by: Gilles DOFFE <[email protected]>
Fix:
  error: ISO C forbids empty initializer braces [-Werror=pedantic]

Signed-off-by: Gilles DOFFE <[email protected]>
Give priority to RIOT_APPLICATION macro over __linux__ and __WIN32__
ones.

Signed-off-by: Gilles DOFFE <[email protected]>
size_t size is changing according to architecture.
Fix diplay to 32 bits unsigned integer.

Signed-off-by: Gilles DOFFE <[email protected]>
Fix:
drivers/sht2x/include/sht2x_params.h:134:50: error: missing binary
operator before token "("

    Signed-off-by: Gilles DOFFE <[email protected]>
This fixes following errors using native architecture:
  RIOT/drivers/pca9685/pca9685.c:42:15: error: format ‘%p’ expects
  argument of type ‘void *’, but argument 5 has type ‘const
  pca9685_params_t *’ [-Werror=format=]

Signed-off-by: Gilles DOFFE <[email protected]>
\ No newline at end of file
+// *INDENT-ON*
+
--
Copy link
Contributor

Choose a reason for hiding this comment

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

Why are you 'fixing' this? This just makes the package harder to maintain.

@benpicco
Copy link
Contributor

benpicco commented Feb 11, 2025

I'm surprised by all the churn.
Stuff like binary constants or empty initializes (both C23) works just fine for me on native.

Try a rebase - we are no longer building native with -pedantic

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: build system Area: Build system Area: cpu Area: CPU/MCU ports Area: drivers Area: Device drivers Area: pkg Area: External package ports Area: SAUL Area: Sensor/Actuator Uber Layer Area: sys Area: System Area: tests Area: tests and testing framework Area: toolchain Area: toolchains; everything related to compilation, libc, linking, … CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Platform: native Platform: This PR/issue effects the native platform Reviewed: 1-fundamentals The fundamentals of the PR were reviewed according to the maintainer guidelines Reviewed: 2-code-design The code design of the PR was reviewed according to the maintainer guidelines Reviewed: 3-testing The PR was tested according to the maintainer guidelines Reviewed: 4-code-style The adherence to coding conventions by the PR were reviewed according to the maintainer guidelines Reviewed: 5-documentation The documentation details of the PR were reviewed according to the maintainer guidelines
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants