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

[WIP] Lidar Lite multi instance #12864

Closed
wants to merge 8 commits into from
Closed

Conversation

DanielePettenuzzo
Copy link
Contributor

@DanielePettenuzzo DanielePettenuzzo commented Sep 2, 2019

With this pr I would like to add support for multiple lidar lites. The current state of this branch is that it works with multiple lidar lites on different I2C busses. I had to pull in #12756 from @cmic0.

TODO:

  • assign different IDs to each sensor based on bus number and how many sensors there are on the same bus
  • create new GPIO driver to be able to power up each lidar on after the other to be able to assign different I2C addresses for sensors on the same bus (would be cool to have this in a separate driver so that each distance sensor driver can use it)
  • make a new smart startup sequence to handle sensors on the same bus and use this new GPIO driver to reassign addresses
  • find out a good strategy for the PWM interface of this driver since now I commented it out
  • fix stop and status commands

FYI @TSC21 @dagar @cmic0

@DanielePettenuzzo
Copy link
Contributor Author

When #12756 will be merged I will remove the commits from this PR

#define LL40LS_DEVICE_PATH "/dev/ll40ls"
#define LL40LS_DEVICE_PATH_EXT "/dev/ll40ls_ext"
#define LL40LS_DEVICE_PATH_EXT_1 "/dev/ll40ls_ext_1"
#define LL40LS_DEVICE_PATH_EXT_2 "/dev/ll40ls_ext_2"
Copy link
Member

Choose a reason for hiding this comment

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

We don't need these paths anymore.

@dagar
Copy link
Member

dagar commented Sep 2, 2019

find out a good strategy for the PWM interface of this driver since now I commented it out

The PWM interface version of this is so simple (and different) it might as well be a standalone thing.

https://github.com/PX4/Firmware/blob/47355333add4070a13c777737736cf2a351d0dfa/src/drivers/distance_sensor/ll40ls/LidarLitePWM.cpp#L90-L116

@mcsauder mcsauder mentioned this pull request Sep 2, 2019
@TSC21
Copy link
Member

TSC21 commented Sep 4, 2019

@DanielePettenuzzo #12756 was merged. This now can be rebased and continued.

@mcsauder
Copy link
Contributor

Hi @TSC21 , when you get a moment, if you can resolve the current conflicts and rebase I can dig into this PR with you!

@TSC21
Copy link
Member

TSC21 commented Sep 11, 2019

Hi @TSC21 , when you get a moment, if you can resolve the current conflicts and rebase I can dig into this PR with you!

Sure thing. I will try to do that today.

@mcsauder
Copy link
Contributor

@DanielePettenuzzo , I've gone through the proposed changes here and in case it saves you some time de-conflicting this PR, I am fairly confident that I have all of your work here captured in PR #12695

PR #12695 is currently being held back by PR #12934, but if those two PRs get merged all of your work here will be accomplished and then we can begin the work for multiple sensors!

If you have time, would you be willing to try out PR #12695 and let me know if you would like to see anything changed there?

@mcsauder
Copy link
Contributor

Hi @TSC21 , when you get a moment, if you can resolve the current conflicts and rebase I can dig into this PR with you!

@DanielePettenuzzo , I apologize, I see now that I had gotten my wires crossed thinking this was @TSC21 's PR.

Please let me know how I can help!

@mcsauder
Copy link
Contributor

@DanielePettenuzzo , try out current master branch and tell me what else is missing for you... I think it is close now!

@DanielePettenuzzo
Copy link
Contributor Author

@mcsauder awesome. I will try it out some time this week

@stale
Copy link

stale bot commented Dec 27, 2019

This issue has been automatically marked as stale because it has not had recent activity. Thank you for your contributions.

@stale stale bot added the stale label Dec 27, 2019
@TSC21
Copy link
Member

TSC21 commented Dec 27, 2019

This is ongoing work on my side, though not in this PR. This is still relevant.

@stale
Copy link

stale bot commented Mar 26, 2020

This issue has been automatically marked as stale because it has not had recent activity. Thank you for your contributions.

@stale stale bot added the stale label Mar 26, 2020
@mortenfyhn
Copy link
Contributor

Hi everyone! This seems promising, and would be useful for me and I'm sure others. Do you have any updates, or is there another work in progress branch that can be made available? (@TSC21 mentioned you might have something.)

@stale stale bot removed the stale label Apr 15, 2020
@mortenfyhn
Copy link
Contributor

Update: I've been testing a bit, and as far as I can tell, after the recent refactor of some driver stuff, this is already implemented on https://github.com/PX4/Firmware/releases/tag/v1.11.0-beta1

I tested it with two LidarLites of the same type, ran ll40ls start -X in the QGroundControl Mavlink Console, and I got data from both with listener distance_sensor.

@TSC21
Copy link
Member

TSC21 commented Apr 17, 2020

Update: I've been testing a bit, and as far as I can tell, after the recent refactor of some driver stuff, this is already implemented on https://github.com/PX4/Firmware/releases/tag/v1.11.0-beta1

I tested it with two LidarLites of the same type, ran ll40ls start -X in the QGroundControl Mavlink Console, and I got data from both with listener distance_sensor.

@mortenfyhn that is possible if you connect each in different buses, But you can't do more than that. The purpose here was to extend it to work with multiple devices on the same bus, which requires some power enable triggering logic to change each device address.

@mortenfyhn
Copy link
Contributor

Update: I've been testing a bit, and as far as I can tell, after the recent refactor of some driver stuff, this is already implemented on https://github.com/PX4/Firmware/releases/tag/v1.11.0-beta1
I tested it with two LidarLites of the same type, ran ll40ls start -X in the QGroundControl Mavlink Console, and I got data from both with listener distance_sensor.

@mortenfyhn that is possible if you connect each in different buses, But you can't do more than that. The purpose here was to extend it to work with multiple devices on the same bus, which requires some power enable triggering logic to change each device address.

Thanks for clarifying, that is indeed what I did. Good to know.

@w407022008
Copy link

Hey guys. So there is any solution now? I'd like to kown how do we drive multiple same sensors at different address but on the same I2C bus by using the identical driver? Thx!

@dagar
Copy link
Member

dagar commented Jul 12, 2020

Hey guys. So there is any solution now? I'd like to kown how do we drive multiple same sensors at different address but on the same I2C bus by using the identical driver? Thx!

The drivers themselves will handle multiple instances, but you still need a mechanism to get them on separate I2C addresses (or use different buses).

@w407022008
Copy link

w407022008 commented Jul 13, 2020

@dagar Thx Dagar!
Actually yes, they will, but what i confused is how could we offer such a mechanism to get them on separate I2C addresses. After all, there are merely 3 external I2C ports for Pixhawk 4 for example, so i couldn't have enough buses for driving 5 sensors on different buses right? More details, i have 5 distance sensors of the same kind with facing different directions, and therefore they could be started by the same driver i hope. However, for example in sf1xx.cpp,

SF1XX* instance = new SF1XX(iterator.configuredBusOption(), iterator.bus(), cli.orientation, cli.bus_frequency);

there we can just declare one instance once we command sf1xx start -X -R 25. What i want to ask is how will you write a program (MODULE drivers__)(e.g: sf1xx.cpp) which could declare multiple instances and power these sensors once the driver is started by command just once?

Thx again!
Ze

@davids5
Copy link
Member

davids5 commented Jul 13, 2020

@dagar Thx Dagar!
Actually yes, they will, but what i confused is how could we offer such a mechanism to get them on separate I2C addresses.

https://www.sparkfun.com/products/16784

@w407022008
Copy link

@davids5 Thx David! Yes, i think so. The multiplexer is a good choice to those sensor with unmodifiable I2C address. Well, the sensors which i have support modifying I2C address, and i have modified them.
The trouble is: if we have 5 same model sensors with different I2C address, if we connect them on the same I2C bus (for ex I2C4 port of pixhawk) by using a splitter, how should we write a driver for driving these sensors? In other words, is it possible that we could just create only one driver module for starting simultaneously the sensors on separate I2C addresses by issuing only once a command (e.g driver_name start -X ) in mavlink console? Or, we must create a driver for each sensor even though they are the same model, correct?
I will be very grateful, if you give me some ideas.
Thx again!
Ze

@davids5
Copy link
Member

davids5 commented Jul 13, 2020

@davids5 Thx David! Yes, i think so. The multiplexer is a good choice to those sensor with unmodifiable I2C address. Well, the sensors which i have support modifying I2C address, and i have modified them.
The trouble is: if we have 5 same model sensors with different I2C address, if we connect them on the same I2C bus (for ex I2C4 port of pixhawk) by using a splitter, how should we write a driver for driving these sensors? In other words, is it possible that we could just create only one driver module for starting simultaneously the sensors on separate I2C addresses by issuing only once a command (e.g driver_name start -X ) in mavlink console? Or, we must create a driver for each sensor even though they are the same model, correct?
I will be very grateful, if you give me some ideas.
Thx again!
Ze

If the options are available on all HW (not just yours) it would be of value to the community.

Have a look at the some of the sensor driver that take a bus and address. Model it after that.

driver start -b 1 -a 0x21 # device at 0x21 on I2C bus 1
driver start -b 1 -a 0x22 # device at 0x22 on I2C bus 1
... or 
driver start -b 1 -X  # all on external I2C bus 1

@w407022008
Copy link

@davids5 Thx David! I will spend some time trying it.

@stale
Copy link

stale bot commented Dec 25, 2020

This issue has been automatically marked as stale because it has not had recent activity. Thank you for your contributions.

@stale stale bot added the stale label Dec 25, 2020
@LorenzMeier
Copy link
Member

Closing as stale.

@dagar dagar deleted the pr-lidarlite-multi3 branch October 9, 2022 19:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants