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.

Sign up for GitHub

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

Jump to bottom

drivers: I2CSPIDriver add a number of missing addresses #14679

Merged
merged 1 commit into from
Apr 17, 2020
Merged

drivers: I2CSPIDriver add a number of missing addresses #14679

merged 1 commit into from
Apr 17, 2020

Conversation

dagar
Copy link
Member

@dagar dagar commented Apr 16, 2020

No description provided.

@dagar dagar requested a review from bkueng April 16, 2020 03:28
bkueng
bkueng reviewed Apr 16, 2020
View reviewed changes
Copy link
Member

@bkueng bkueng left a comment

Choose a reason for hiding this comment

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

I don't think it's enough to only add it here. Either you also make it configurable, or relax the logic in https://github.com/PX4/Firmware/blob/master/platforms/common/i2c_spi_buses.cpp#L274 (_i2c_address will be 0).

@SalimTerryLi
Copy link
Contributor

SalimTerryLi commented Apr 16, 2020 •
edited
Loading

How about make constructor requests for address as well, instead of an optional parameter?

@dagar
Copy link
Member Author

dagar commented Apr 16, 2020

This was just a quick attempt to silence distracting console output before the next release.

[2020-04-16T10:00:16.987Z] ERROR [SPI_I2C] Bug: driver rgbled does not pass the I2C address to I2CSPIDriverBase
[2020-04-16T10:00:16.987Z] rgbled #0 on I2C bus 2 (external)

@bkueng
Copy link
Member

bkueng commented Apr 17, 2020

This was just a quick attempt to silence distracting console output before the next release.

That is indeed wrong then. I was looking at LidarLiteI2C.
Going through them, only blinkm, rgbled_ncp5623c and rgbled need to be fixed.

@dagar
Copy link
Member Author

dagar commented Apr 17, 2020

Updated to only blinkm, rgbled_ncp5623c and rgbled.

@dagar
Copy link
Member Author

dagar commented Apr 17, 2020

Sweet silence (or at least slightly improved).

Screenshot from 2020-04-17 11-52-24

@dagar dagar merged commit d9dfedb into master Apr 17, 2020
@dagar dagar deleted the pr-rgbled_fix branch April 17, 2020 15:54
@weekly-digest weekly-digest bot mentioned this pull request Apr 19, 2020
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bkueng bkueng bkueng left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@dagar @SalimTerryLi @bkueng