Skip to content

drivers/hts221: adapt to i2c api return codes#9298

Merged
aabadie merged 1 commit intoRIOT-OS:new_i2c_iffrom
aabadie:new_i2c_hts221
Jun 28, 2018
Merged

drivers/hts221: adapt to i2c api return codes#9298
aabadie merged 1 commit intoRIOT-OS:new_i2c_iffrom
aabadie:new_i2c_hts221

Conversation

@aabadie
Copy link
Copy Markdown
Contributor

@aabadie aabadie commented Jun 6, 2018

Contribution description

This PR readapts the hts221 driver to the I2C return codes (< 0 in case of an error, 0 on success).

Issues/PRs references

Follow-up of #9195 and related to #6577

@aabadie aabadie added Area: drivers Area: Device drivers TF: I2C Marks issues and PRs related to the work of the I²C rework task force labels Jun 6, 2018
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.

Overall changes are OK,
Could you share some debug output or do you know if someone can test this device so we can move forward ?
BTW, why acquire/release are used several times in the init function ? never saw this behaviour.

@aabadie
Copy link
Copy Markdown
Contributor Author

aabadie commented Jun 21, 2018

why acquire/release are used several times in the init function ?

I looked at the code more in depth, and this is perfectly fine in term of workflow: it's just that some private functions are not acquiring the bus before performing their read/write operations.

here is an output with a nucleo-l073:

2018-06-21 11:25:57,327 - INFO # Connect to serial port /dev/ttyACM0
Welcome to pyterm!
Type '/exit' to exit.
2018-06-21 11:25:58,801 - INFO # H: 56.4%, T: 24.8°C
2018-06-21 11:26:00,805 - INFO # H: 56.1%, T: 24.8°C
2018-06-21 11:26:02,803 - INFO # H: 56.0%, T: 24.8°C
2018-06-21 11:26:04,807 - INFO # H: 55.9%, T: 24.8°C

@dylad
Copy link
Copy Markdown
Member

dylad commented Jun 21, 2018

I looked at the code more in depth, and this is perfectly fine in term of workflow: it's just that some private functions are not acquiring the bus before performing their read/write operations.

Sure, but this is first time I saw this. Is there any gain from doing this ? I don't have strong opinion about this. If the driver was already in this state before, this is not our rights to change it with the I2C refactoring.
Anyway, it would be great if someone could test it, otherwise I can leave an untested ACK.

@aabadie
Copy link
Copy Markdown
Contributor Author

aabadie commented Jun 22, 2018

it would be great if someone could test it

I think @smlng has the hardware, since he initially wrote this driver. Maybe he can comment on your other questions.

@aabadie aabadie merged commit c20a6fe into RIOT-OS:new_i2c_if Jun 28, 2018
basilfx pushed a commit to basilfx/RIOT that referenced this pull request Jul 10, 2018
drivers/hts221: adapt to i2c api return codes
dylad pushed a commit to dylad/RIOT that referenced this pull request Jul 10, 2018
drivers/hts221: adapt to i2c api return codes
dylad pushed a commit that referenced this pull request Jul 11, 2018
drivers/hts221: adapt to i2c api return codes
@aabadie aabadie deleted the new_i2c_hts221 branch August 31, 2018 07:05
@aabadie aabadie added this to the Release 2018.10 milestone Nov 5, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

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

Development

Successfully merging this pull request may close these issues.

3 participants