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

Lidarlite merge and bringup #2224

Merged
merged 63 commits into from
May 24, 2015
Merged

Lidarlite merge and bringup #2224

merged 63 commits into from
May 24, 2015

Conversation

bansiesta
Copy link
Contributor

This branch brings the WIP PRs #1949 and #2196 to a working state.
Also merged now is #2200.

The PR also contains: #2222

Feel free to review, comment and test, thanks.

Things tested:

  • pwm_input start
  • pwm_input test
  • ll40ls start pwm
  • ll40ls test pwm
  • DISTANCE_SENSOR message in QGroundControl
nsh> pwm_input start
nsh> ll40ls start pwm
nsh> ll40ls test pwm
ll40ls: single read
ll40ls: measurement: 1.81 m
ll40ls: time:        0
ll40ls: periodic read 0
ll40ls: measurement: 1.842 m
ll40ls: time:        0
ll40ls: periodic read 1
ll40ls: measurement: 1.842 m
ll40ls: time:        0
ll40ls: periodic read 2
ll40ls: measurement: 1.818 m
ll40ls: time:        0
ll40ls: periodic read 3
ll40ls: measurement: 1.807 m
ll40ls: time:        0
ll40ls: periodic read 4
ll40ls: measurement: 1.836 m
ll40ls: time:        0
ll40ls: PASS
nsh> ll40ls info pwm
state @ 20014c40
ll40ls_pwm_read: 218 events, 0 overruns, 12104us elapsed, 55us avg, min 34us max 2471us 165.817us rms
ll40ls_pwm_read_errors: 1 events
ll40ls_pwm_buffer_overflows: 208 events
ll40ls_pwm_zero_resets: 1 events
ll40ls: poll interval:  50 ticks
ll40ls: distance: 1.839m

Also, the reset on Aux pin 6 seems to be working:

nsh> Idle Task: Lidar is down, reseting

Zefz and others added 30 commits March 24, 2015 10:04
publication to range_finder topic added
@TSC21
Copy link
Member

TSC21 commented May 24, 2015

@LorenzMeier probably better check if QGC gets all the range sensors data plotted. Then probably we can merge. Still, we probably have to work first in a multi pub/sub implementation.

@bansiesta
Copy link
Contributor Author

@LorenzMeier: I'll update the wiki instructions when it is merged. I would like if you could review the part here #2224 (diff)

@mhkabir
Copy link
Member

mhkabir commented May 24, 2015

@TSC21 Did you modify the flow driver to publish the sonar to a ranger topic? As-is, there isn't anything of that sort, so you won't get any data out.

@TSC21
Copy link
Member

TSC21 commented May 24, 2015

@mhkabir
Copy link
Member

mhkabir commented May 24, 2015

@TSC21 Oh no! We deprecated the serial interface quite some time back. Actively supported is only i2c.
Driver here : https://github.com/PX4/Firmware/tree/master/src/drivers/px4flow
Please make the changes and test :) Thanks!!

@TSC21
Copy link
Member

TSC21 commented May 24, 2015

Yes I was talking with @bansiesta at gitter now about this. He's already getting around that

@bansiesta
Copy link
Contributor Author

The PX4Flow sonar distance is now published to distance_sensor as well, however, the multi pub/sub does not seem to work yet. It will need some changes.
@LorenzMeier pointers welcome :)

@TSC21 TSC21 mentioned this pull request May 24, 2015
@TSC21
Copy link
Member

TSC21 commented May 24, 2015

The PX4Flow sonar distance is now published to distance_sensor as well, however, the multi pub/sub does not seem to work yet. It will need some changes.

Confirmed :D One idea for multi sub/pub: 571457a

@LorenzMeier
Copy link
Member

You need to use orb_publish_multi()

@bansiesta
Copy link
Contributor Author

I've added orb_advertise_multi in all distance sensors now, however, the modules using the data are all subscribing to the default (0).

I've tried the lidarlite together with a PX4Flow connected:

nsh> ls /dev
...
range_finder0
range_finder1
...

@TSC21
Copy link
Member

TSC21 commented May 24, 2015

@bansiesta so how can we choose what structure each module subscribe to?

@TSC21
Copy link
Member

TSC21 commented May 24, 2015

I was checking, we probably have to use orb_subscribe_multi() in every module, depending on the sensor instance.

Note: In this case, if we then use serial interface, do we have to create a mavlink stream for each sensor?

@bansiesta
Copy link
Contributor Author

Note: In this case, if we then use serial interface, do we have to create a mavlink stream for each sensor?

I assume so. For now though, we can leave it as is and merge it, in my opinion. If later a module needs to subscribe to more than one, it can/should do so.

@TSC21
Copy link
Member

TSC21 commented May 24, 2015

Ekf_attitude_position estimator does need more than one range finder, as it's also defined in code. Also, how can sensors be distinguished in a telemetry link and be presented separatly in QGC for example?

@TSC21
Copy link
Member

TSC21 commented May 24, 2015

Anyhow, we also should consider the ID to get to each one. I say that we should reserve like an array of IDs for each type of sensor. For example, 0 to 10 for sonar sensors and 11 to 20 to lidar (assuming of course that someone would connect 20 range finders to the BUS xD). Not sure the max size of the I2C bus though, but the current serial link can allow to retrieve sensor data from sensors connected to companion computers.

@TSC21
Copy link
Member

TSC21 commented May 24, 2015

Note: topic names should correspond to id of the sensor, which should correspond to the instance id of 'range_finderN', meaning 'distance_sensorN', with N=distance_sensor.id=N from 'range_finderN'. That way we can have a much more organized topic presentation and will allow multi sub/pub. Also we must consider sensor data coming from the serial stream and organize it the same way as the ones connected to the I2C bus.

@LorenzMeier
Copy link
Member

Nice! Merging.

LorenzMeier added a commit that referenced this pull request May 24, 2015
@LorenzMeier LorenzMeier merged commit c5f14ef into PX4:master May 24, 2015
@LorenzMeier
Copy link
Member

@TSC21 If you want to identify a particular sensor, send the sensor ID as part of the message, not as part of the topic name. Then the subscriber can pick the one it prefers based on ID / specs.

@bansiesta bansiesta deleted the lidarlite_merge branch May 24, 2015 21:22
@TSC21
Copy link
Member

TSC21 commented May 24, 2015

@LorenzMeier and how can that be done at QGC/sdlog2 level for example? Cause in mavros implementation I already can.

@TSC21
Copy link
Member

TSC21 commented May 24, 2015

Also, I don't think this is complete while the multi subscription is not implemented. It doesn't make sense to have multi advertising without having multisubscription, having estimators requiring more than one range source (which is the case of ekf_att_pos_estimator).

@bansiesta
Copy link
Contributor Author

Note: documentation updated here: https://pixhawk.org/peripherals/rangefinder?#pwm_usage

@jgoppert
Copy link
Member

@bansiesta

I have it wired and reading via test, but daemon process fails to start.

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.

6 participants