Skip to content

drivers: bmx280: adapt to new I2C api.#9206

Merged
aabadie merged 1 commit intoRIOT-OS:new_i2c_iffrom
basilfx:feature/i2c_drivers_bmx280
Jun 19, 2018
Merged

drivers: bmx280: adapt to new I2C api.#9206
aabadie merged 1 commit intoRIOT-OS:new_i2c_iffrom
basilfx:feature/i2c_drivers_bmx280

Conversation

@basilfx
Copy link
Copy Markdown
Member

@basilfx basilfx commented May 27, 2018

Contribution description

This PR updates the BMx280 driver. Tested this one on the SLTB001A (EFM32, adapted).

It's probably better to wait for #8383, but it was easy to adapt for the time being.

Issues/PRs references

#6577

@basilfx basilfx added the TF: I2C Marks issues and PRs related to the work of the I²C rework task force label May 27, 2018
@basilfx basilfx requested a review from dylad May 27, 2018 20:44
@haukepetersen haukepetersen mentioned this pull request May 27, 2018
Closed
47 tasks
@basilfx basilfx mentioned this pull request May 27, 2018
Merged
@aabadie aabadie added the Area: drivers Area: Device drivers label Jun 3, 2018
@aabadie aabadie mentioned this pull request Jun 7, 2018
Merged
aabadie
aabadie requested changes Jun 7, 2018
View reviewed changes
Copy link
Copy Markdown
Contributor

@aabadie aabadie left a comment

Choose a reason for hiding this comment

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

Needs an update to latest API.

Otherwise I tested this PR with #9198 on an Arduino Zero board and it works.

drivers/bmx280/bmx280.c Outdated
result = i2c_read_regs(dev->params.i2c_dev, dev->params.i2c_addr, offset,
buffer, nr_bytes_to_read, 0);
i2c_release(dev->params.i2c_dev);
if (result != I2C_ACK) {
Copy link
Copy Markdown
Contributor

@aabadie aabadie Jun 7, 2018

Choose a reason for hiding this comment

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

I2C_ACK 'return code was removed from the API, prefer a check like != or <0.

drivers/bmx280/bmx280.c Outdated
result = i2c_read_regs(dev->params.i2c_dev, dev->params.i2c_addr,
offset, measurement_regs, nr_bytes_to_read, 0);
i2c_release(dev->params.i2c_dev);
if (result != I2C_ACK) {
Copy link
Copy Markdown
Contributor

@aabadie aabadie Jun 7, 2018

Choose a reason for hiding this comment

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

Same here.

@keestux
Copy link
Copy Markdown
Contributor

keestux commented Jun 9, 2018

@basilfx can you rebase onto the latest new_i2c_if?

@basilfx basilfx force-pushed the feature/i2c_drivers_bmx280 branch from 77e3618 to ec65df2 Compare June 17, 2018 08:50
@basilfx
Copy link
Copy Markdown
Member Author

basilfx commented Jun 17, 2018

Sorry for the delay. I have rebased and adapted, but I cannot test this one since I have to fix EFM32 first (which will take some time).

@keestux
Copy link
Copy Markdown
Contributor

keestux commented Jun 17, 2018

Thanks
I've tested it with SODAQ Autonomo, and I confirm it is working

dylad
dylad reviewed Jun 17, 2018
View reviewed changes
Copy link
Copy Markdown
Member

@dylad dylad left a comment

Choose a reason for hiding this comment

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

I'm a bit puzzled because sometimes return codes are used and rest of the time you used (void) function and you're returning other data.
As I know, the coding lines did not say anything about that but this is a bit inconsistent from my point of view.
@aabadie what do you think ?

@basilfx
Copy link
Copy Markdown
Member Author

basilfx commented Jun 17, 2018

@dylad Can you tell me which lines? Note that I only adapted this driver to make it work with the new interface, so most code was already in place.

dylad
dylad approved these changes Jun 18, 2018
View reviewed changes
Copy link
Copy Markdown
Member

@dylad dylad left a comment

Choose a reason for hiding this comment

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

Note that I only adapted this driver to make it work with the new interface, so most code was already in place.

@basilfx You're right, the aim of the refactoring is only to adapt the current driver.
Since @keestux successfully tested this PR, I guess we can go :)
I give you my ACK !

@basilfx basilfx force-pushed the feature/i2c_drivers_bmx280 branch from ec65df2 to 1393890 Compare June 18, 2018 17:15
@basilfx
Copy link
Copy Markdown
Member Author

basilfx commented Jun 18, 2018

Squashed.

@dylad
Copy link
Copy Markdown
Member

dylad commented Jun 18, 2018

@aabadie Can we dismiss your review ?

aabadie
aabadie approved these changes Jun 19, 2018
View reviewed changes
Copy link
Copy Markdown
Contributor

@aabadie aabadie left a comment

Choose a reason for hiding this comment

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

My requested changes were addressed.

ACK and go

@aabadie aabadie merged commit ac26342 into RIOT-OS:new_i2c_if Jun 19, 2018
basilfx pushed a commit to basilfx/RIOT that referenced this pull request Jul 10, 2018
@aabadie @basilfx
Merge pull request RIOT-OS#9206 from basilfx/feature/i2c_drivers_bmx280
45959b1 
drivers: bmx280: adapt to new I2C api.
dylad pushed a commit to dylad/RIOT that referenced this pull request Jul 10, 2018
@aabadie @dylad
Merge pull request RIOT-OS#9206 from basilfx/feature/i2c_drivers_bmx280
4700cdd 
drivers: bmx280: adapt to new I2C api.
dylad pushed a commit that referenced this pull request Jul 11, 2018
@aabadie @dylad
Merge pull request #9206 from basilfx/feature/i2c_drivers_bmx280
baf47a5 
drivers: bmx280: adapt to new I2C api.
@basilfx basilfx deleted the feature/i2c_drivers_bmx280 branch January 14, 2020 22:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@aabadie aabadie aabadie approved these changes

@dylad dylad dylad approved these changes

Assignees

No one assigned

Labels

Area: drivers Area: Device drivers TF: I2C Marks issues and PRs related to the work of the I²C rework task force

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@basilfx @keestux @dylad @aabadie @smlng