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

Deal with i2c address conflict - Add documentaiton comment #2

Merged
merged 11 commits into from
Dec 29, 2017

Conversation

Ybalrid
Copy link
Contributor

@Ybalrid Ybalrid commented Dec 2, 2017

Hi, I'm using this library in a student project, but I needed to do some small modifications.

We are using 2 of theses sensors on the same i²c bus. This is doable by pulling down the "chip enable" pin of one, while asking the other to change it's address. There's a writable register for that described into the datasheet at address 0x0212.

Also, in the case you are working after a program did configured the sensor at another address, you can get into some issues. The initialize function now get the address to use as a parameter. The default one is #defined into the main header

I also added comments in Doxygen syntax so a documentation can be generated from the header. Even if it's a really simple library this can be useful.

I'm planning more changes to do on this library to prevent some name collision, all public functions should have the "vl6180_". I'm probably going to PR that real soon (getting back to school next week and I'll have the time to work on this)

This is not the cleanest of patches as my editor auto-indented the code so all the white-space in the c file has been changed, but since theses changes are useful to us I wanted to share them ;-)

@Ybalrid
Copy link
Contributor Author

Ybalrid commented Dec 2, 2017

Oh, I also forgot to tell, I edited the makefile so the library install itself into /usr/local and not /usr/ by default. I'm not comfortable with putting "external" not tracked by the package manager into these directories, and that the purpose of the "local" one. You can edit that out if it's problematic ^^"

/// \param device The I2C bus to open. e.g. "1" for using /dev/i2c-1
/// \param i2c_addr addres of the device. If you don't know, pass VL1680_DEFALUT_ADDR to it
/// \return handle to the sensor. Keep this variable to talk to the sensor via the library
vl6180 vl6180_initialise(int device, int i2c_addr);
Copy link
Owner

Choose a reason for hiding this comment

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

i would like to keep this backwards compatible if possible. Can you add another initialise function that defaults to the defined address

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fair enough.

What I'm gonna do is rename this function "vl6480_initialise_addr", and keep the "vl6480_initialise" with the old interface, and make it call this one with default address that is now defined in the header.

I will update the PR shortly ;-)

@leachj
Copy link
Owner

leachj commented Dec 5, 2017

Hi

Thanks for the PR. I too was using this library with multiple sensors but we had an i2c multiplexer and used different i2c busses to separate out the sensors. Ive made just one comment about the code, if you happy to fix this i will merge in when your done.

Thanks

Jon

This prevent to break compatibility with older use of the libary
@Ybalrid
Copy link
Contributor Author

Ybalrid commented Dec 6, 2017

@leachj I've updated the PR now, the compatibility isn't broken anymore ;-)

@leachj leachj merged commit 5502ec0 into leachj:master Dec 29, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants