drivers/periph_i2c: let i2c_acquire return void#17275
drivers/periph_i2c: let i2c_acquire return void#17275benpicco merged 6 commits intoRIOT-OS:masterfrom
Conversation
gschorcht
requested review from
DipSwitch,
MichelRottleuthner,
MrKevinWeiss,
aabadie,
basilfx,
benpicco,
bergzand,
dylad,
fjmolinas,
haukepetersen,
kYc0o,
kaspar030,
keestux,
leandrolanzieri,
maribu,
smlng and
vincent-d
as code owners
github-actions
bot
added
Area: arduino API
gschorcht
removed
Platform: ARM
gschorcht
added
the
CI: ready for build
gschorcht
force-pushed
the
drivers/periph_i2c_acquire_void
branch
6 times, most recently
from
0c466ca to
47d4383
Compare
drivers/lis2dh12/lis2dh12.c
Outdated
| /* enable all axes, set sampling rate and scale */ | ||
| LIS2DH12_CTRL_REG1_t reg1 = {0}; | ||
|
|
||
| /* cppcheck-suppress unreadVariable; (reason: union type is used for read) */ |
There was a problem hiding this comment.
@maribu Here we have no volatile variable definitions, but cppcheck complains anyway. So it looks like cppcheck does not recognize the type unit after all. Missing header files are probably not the problem in this case.
There was a problem hiding this comment.
@maribu I played a bit with cppcheck and the single file that made the problems using command:
cppcheck --std=c99 --enable=style --force --error-exitcode=2 --quiet -j 1 --inline-suppr \
--suppress=unusedStructMember --template '{file}:{line}: {severity} ({id}): {message}' \
drivers/lis2dh12/lis2dh12.c
cppcheck version 1.82 (local Ubuntu 18.04 LTS version) doesn't show any warnings.
cppcheck version 1.90 in RIOT docker shows these 32 warnings as in CI.
cppcheck version 2.6 (newest version from sourgeforge.net) shows only 5 warnings.
Adding the required include pathes
-I drivers/include -I drivers/lis2dh12/include/
doesn't help with cppcheck version 1.90 but solves all warnings with cppcheck version 2.6.
So it seems that we need correct include pathes as well as a newer cppcheck version in CI.
drivers/lis2dh12/lis2dh12.c
Outdated
| /* switch to Bypass mode */ | ||
| _write(dev, REG_FIFO_CTRL_REG, fifo_reg.reg); | ||
|
|
||
| /* cppcheck-suppress redundantAssignment; (reason: union type is used for read) */ |
There was a problem hiding this comment.
@maribu Here I don't know how to solve the cppcheck error.
If I use cppcheck-suppress redundantAssignment, cppcheck is complaining about unread variable. If I replace cppcheck-suppress redundantAssignment by cppcheck-suppress unreadVariable, it is complaining about redundant assignment.
How can this be solved?
There was a problem hiding this comment.
Solved. I just have to use multiple cppcheck-suppress lines for all different warning that are given here.
|
Failed run tests are not related to this PR. Therefore, I remove the |
gschorcht
removed
the
CI: run tests
gschorcht
force-pushed
the
drivers/periph_i2c_acquire_void
branch
from
47d4383 to
3f6024c
Compare
Since all implementations simply return 0 and most drivers do not check the return value, it is better to return void and use an assert to ensure that the given device identifier and given device parameters are correct.
Make all `spi_acquire` implementations return `void` and add assertions to check for valid device identifier where missing.
gschorcht
force-pushed
the
drivers/periph_i2c_acquire_void
branch
from
3f6024c to
b7dda22
Compare
|
Thanks |
See [1]. [1] RIOT-OS/RIOT#17275 Signed-off-by: Gilles DOFFE <[email protected]>
See [1]. [1] RIOT-OS/RIOT#17275 Signed-off-by: Gilles DOFFE <[email protected]>
See [1]. [1] RIOT-OS/RIOT#17275 Signed-off-by: Gilles DOFFE <[email protected]>
See [1]. [1] RIOT-OS/RIOT#17275 Signed-off-by: Gilles DOFFE <[email protected]>
4 participants
Contribution description
As follow-up of PR #15902 and based on the discussion in issue #10673 this PR
int i2c_acquire(...)tovoid i2c_acquire(...)becauseassert()where it is missing to ensure that a correct I2C device identifier is used<driver>_init().Testing procedure
Compilation and tests in Murdock should work.
Issues/PRs references
Follow-up of #15902
Fixes #10673