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

SF30/d lidar support #20809

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

SF30/d lidar support #20809

wants to merge 3 commits into from

Conversation

ryanjAA
Copy link
Contributor

@ryanjAA ryanjAA commented Dec 21, 2022

This adds support for the lightware SF30/d lidar (laser altimeter) with 200m range.

Bench tested and works as intended.

It outputs 250m when out of range (and the SF11/c outputs 650m) so we may want to filter those exact numbers out by anything that is consuming this lidar data.

Also, I have this set at 39hz but it can go up to 20k hz with little to no range degradation. Is there a reason to not increase the this (ie is it useful to have faster refresh rate and can that fast data data even be consumed? Concerns here would be overloading the i2c bus, if relevant).

https://www.documents.lightware.co.za/SF30%20D%20-%20LiDAR%20Datasheet%20-%20Rev%200.pdf

Adds support for the SF30/D
Parameter update to include SF30/d
@hamishwillee
Copy link
Contributor

Cool. When this goes in, can you do a PR to update http://docs.px4.io/main/en/sensor/sfxx_lidar.html ?

It would be good if you could check the whole doc - it is old. If you're feeling particularly charming, also update the hardware connection diagram: http://docs.px4.io/main/en/sensor/sfxx_lidar.html#hardware

Especially if it comes with a cable so you don't have to do anything with new Pixhawks.

@ryanjAA
Copy link
Contributor Author

ryanjAA commented Dec 21, 2022

@hamishwillee no prob. Will go through it all.

@hamishwillee
Copy link
Contributor

@ryanjAA Re #20809 (comment) - great! Any progress on this one?

@junwoo091400 junwoo091400 added Drivers 🔧 Sensors, Actuators, etc Admin: Documentation needed PRs that requires Documentation updates labels Mar 24, 2023
@ryanjAA
Copy link
Contributor Author

ryanjAA commented Apr 28, 2023

Test flew this today. Logs to follow. No issues.

@ryanjAA
Copy link
Contributor Author

ryanjAA commented May 2, 2023

@ryanjAA
Copy link
Contributor Author

ryanjAA commented May 12, 2023

Just an interesting thing to note about this unit. If you have an MS4525 airspeed (aka the commonly used old one), your i2c bus will shutdown. It is likely a hardware issue. We tried quite a few of them and different internal revisions of the SF30. This should just be in the docs when (when this gets pulled in) but we've done quite a few flights with this hardware and with different revisions of it.

@hamishwillee just so this is on your radar but i'm happy to do the docs on this.

@kibidev
Copy link
Contributor

kibidev commented Jun 6, 2023

Just an interesting thing to note about this unit. If you have an MS4525 airspeed (aka the commonly used old one), your i2c bus will shutdown. It is likely a hardware issue. We tried quite a few of them and different internal revisions of the SF30. This should just be in the docs when (when this gets pulled in) but we've done quite a few flights with this hardware and with different revisions of it.

@ryanjAA We have had some problem with LW20/C and MS4525 on the same bus. In our case I identified the problem to be these line:
https://github.com/PX4/PX4-Autopilot/blob/cd485b3a13cb4be6e18b31f2485c16298f68a2ae/platforms/nuttx/src/px4/common/px4_init.cpp#LL146C1-L158C36

The LW20/C does not handle the I2C SW reset all command well, and garbles up the bus afterwards. I suspect the MS4525 only changes the startup timing slightly, which can cause the problem to appear or not.

If you have the opportunity, you could try to just remove the lines from your version of PX4, and see if the problem persists. It is what we have done in our fork.

Thanks for this PR, by the way. We will receive a SF30/D soon, I will test it then and report any findings.

kibidev pushed a commit to aviant-tech/PX4-Autopilot that referenced this pull request Jun 6, 2023
@ryanjAA
Copy link
Contributor Author

ryanjAA commented Jun 6, 2023

Just an interesting thing to note about this unit. If you have an MS4525 airspeed (aka the commonly used old one), your i2c bus will shutdown. It is likely a hardware issue. We tried quite a few of them and different internal revisions of the SF30. This should just be in the docs when (when this gets pulled in) but we've done quite a few flights with this hardware and with different revisions of it.

@ryanjAA We have had some problem with LW20/C and MS4525 on the same bus. In our case I identified the problem to be these line: https://github.com/PX4/PX4-Autopilot/blob/cd485b3a13cb4be6e18b31f2485c16298f68a2ae/platforms/nuttx/src/px4/common/px4_init.cpp#LL146C1-L158C36

The LW20/C does not handle the I2C SW reset all command well, and garbles up the bus afterwards. I suspect the MS4525 only changes the startup timing slightly, which can cause the problem to appear or not.

If you have the opportunity, you could try to just remove the lines from your version of PX4, and see if the problem persists. It is what we have done in our fork.

Thanks for this PR, by the way. We will receive a SF30/D soon, I will test it then and report any findings.

Cool. We’ll try it today or tomorrow. Have you flown it?

@ryanjAA
Copy link
Contributor Author

ryanjAA commented Jun 8, 2023

@kibidev Confirmed that fixes it - not flight tested though. Checked with SF30/d, MS4525do and RM3100 on same bus.

@kibidev
Copy link
Contributor

kibidev commented Jun 26, 2023

@ryanjAA
We have tested this now (cube orange, VTOL drone). Works great, tested to up to ~100m altitude, over green fields.

I needed to connect with usb and set the correct legacy mode for the sensor to work. This is not the same procedure as currently described in the documentation for other Lightware sensors (unsure if this is because the sensor is difference, or the current documentation is outdated), so great if you are able to update documentation as well.

Regarding update rate, we do not need any higher rate at least. If a higher rate should be added, I think it should be configurable, so it is possible to limit the I2C traffic, and PX4 resource usage in general.

@ryanjAA
Copy link
Contributor Author

ryanjAA commented Jul 16, 2023

@kibidev

Just an interesting thing to note about this unit. If you have an MS4525 airspeed (aka the commonly used old one), your i2c bus will shutdown. It is likely a hardware issue. We tried quite a few of them and different internal revisions of the SF30. This should just be in the docs when (when this gets pulled in) but we've done quite a few flights with this hardware and with different revisions of it.

@ryanjAA We have had some problem with LW20/C and MS4525 on the same bus. In our case I identified the problem to be these line: https://github.com/PX4/PX4-Autopilot/blob/cd485b3a13cb4be6e18b31f2485c16298f68a2ae/platforms/nuttx/src/px4/common/px4_init.cpp#LL146C1-L158C36

The LW20/C does not handle the I2C SW reset all command well, and garbles up the bus afterwards. I suspect the MS4525 only changes the startup timing slightly, which can cause the problem to appear or not.

If you have the opportunity, you could try to just remove the lines from your version of PX4, and see if the problem persists. It is what we have done in our fork.

Thanks for this PR, by the way. We will receive a SF30/D soon, I will test it then and report any findings.

Interestingly the bus issue popped back up today. MS4525 airspeed and with the SF30, it doesnt work (and the above commented out), swap to the same MS4525 but made by MRO (which has an LDO on it), it works all together. I tested 12 of these on the bench for good measure. Only the MRO ones work so there is something strange and if you unplug the sf30, all non MRO work fine (aka not the sensor per se).

@ryanjAA
Copy link
Contributor Author

ryanjAA commented Sep 20, 2023

Writing for tracking purposes. Just saw this again today with the init commented out on an MS4525 without an LDO... Strange but since 2 months later and different hardware, maybe an ms4525 batch issue (we buy them in packs of 100 but this is a different batch) MRO ones are working but will buy a non-MRO, non LDO one and write back.

ryanjAA added a commit to ryanjAA/PX4-Autopilot that referenced this pull request Sep 27, 2023
The lightware LW20/C and SF30/d require the i2c reset here to be removed for it to work properly. If you plug in the commonly used MS4525do airspeed without this PR the entire i2c bus stops responding. This fixes it. Been flown without issue since early June.

The LW20/c just doesnt handle the reset command and won't work so this is needed for that as well (see here PX4#20809 (comment))
@ryanjAA
Copy link
Contributor Author

ryanjAA commented Sep 27, 2023

#22149 is required.

@ryanjAA
Copy link
Contributor Author

ryanjAA commented Sep 27, 2023

Writing for tracking purposes. Just saw this again today with the init commented out on an MS4525 without an LDO... Strange but since 2 months later and different hardware, maybe an ms4525 batch issue (we buy them in packs of 100 but this is a different batch) MRO ones are working but will buy a non-MRO, non LDO one and write back.

Batch issue - it doesnt make sense (unplug the SF30/d and it works) but no other explanation...

ryanjAA pushed a commit to ryanjAA/PX4-Autopilot that referenced this pull request Sep 27, 2023
ryanjAA pushed a commit to ryanjAA/PX4-Autopilot that referenced this pull request Nov 22, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Admin: Documentation needed PRs that requires Documentation updates Drivers 🔧 Sensors, Actuators, etc
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants