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

Cannot start ll40ls on I2C bus 4 when specifying -b 4 (v1.11.0-beta1) #14730

Closed
mortenfyhn opened this issue Apr 22, 2020 · 13 comments · Fixed by #14748
Closed

Cannot start ll40ls on I2C bus 4 when specifying -b 4 (v1.11.0-beta1) #14730

mortenfyhn opened this issue Apr 22, 2020 · 13 comments · Fixed by #14748
Assignees
Labels
bug Documentation 📑 Anything improving the documentation of the code / ecosystem

Comments

@mortenfyhn
Copy link
Contributor

When running ll40ls start -X -b 4 a Garmin LidarLite on that bus won't start. When running ll40ls start -X, it does start.

Steps to reproduce the behavior:

  1. Flash PixHawk 4 with v1.11.0-beta1 firmware
  2. Connect a Garmin LidarLite on the I2C A port.
  3. Use QGC's mavlink console, and run ll40ls start -X -b 4.
  4. Nothing happens.

I expect the driver to start and to begin producing distance_sensor data.

I ran this on a PixHawk 4 on my desk, connected to my PC with USB. One/two LidarLites and nothing else connected to the PixHawk.

ll40ls-bug

More details, testing starting both, only one at a time, with lidars on both buses, and on only one bus at a time:

Lidar connected on I2C A and B Lidar connected on I2C B (bus 2) Lidar connected on I2C A (bus 4)
ll40ls start -X Starts on bus 2 and 4 Starts on bus 2 Starts on bus 4
ll40ls start -X -b 2 Starts on bus 2 Starts on bus 2 Nothing starts (expected)
ll40ls start -X -b 4 Nothing starts (unexpected) Nothing starts (expected) Nothing starts (unexpected)
@bkueng
Copy link
Member

bkueng commented Apr 22, 2020

Use ll40ls start -X -b 1. With external buses, the bus numbering should match the label, so you don't have to know about board internals.

@mortenfyhn
Copy link
Contributor Author

mortenfyhn commented Apr 22, 2020

Oh! I didn't realize. Should've tried that. Do you know is this is documented somewhere?

Edit: Just to be sure: Does that mean they are they are numbered in the same order as the naming of the I2C ports on the hardware, so port A is -b 1 and port B is -b 2 and so on?

@mortenfyhn
Copy link
Contributor Author

mortenfyhn commented Apr 22, 2020

And for completeness' sake: I tried -b 1 now and it works perfectly.

@bkueng
Copy link
Member

bkueng commented Apr 22, 2020

ll40ls -h?

Does that mean they are they are numbered in the same order as the naming of the I2C ports on the hardware, so port A is -b 1 and port B is -b 2 and so on?

It should, but not sure if it's correct for all boards. Let me know if you find inconsistencies.

@mortenfyhn
Copy link
Contributor Author

mortenfyhn commented Apr 22, 2020

ll40ls -h?

You're right, it does indeed say

bushelp

After having previously tested

ll40ls start -X

and getting

ll40lsstart

I was certain that the -b option would expect the same numbering as printed here (I'm clearly mistaken).

I think this might be confusing for other people as well. Maybe the ll40ls help output can be modified, or this made clearer in some other way.

As far as I can tell this is new behaviour in v1.11, with v1.10 using the same numbers internally and for the -b switch.

@dagar
Copy link
Member

dagar commented Apr 22, 2020

As far as I can tell this is new behaviour in v1.11, with v1.10 using the same numbers internally and for the -b switch.

It was a mixed bag prior to v1.11. Some drivers implemented this mechanism, some implemented it differently, some only worked on 1 "special" bus. As of v1.11 it should all be handled consistently now (+/- minor bugs per board).

@dagar dagar added bug Documentation 📑 Anything improving the documentation of the code / ecosystem labels Apr 22, 2020
@bkueng
Copy link
Member

bkueng commented Apr 23, 2020

I was certain that the -b option would expect the same numbering as printed here (I'm clearly mistaken).

What if we extend the output to say on I2C bus 4 (1. external bus)?

I think this might be confusing for other people as well. Maybe the ll40ls help output can be modified, or this made clearer in some other way.

What do you suggest?

@mortenfyhn
Copy link
Contributor Author

As far as I can tell this is new behaviour in v1.11, with v1.10 using the same numbers internally and for the -b switch.

It was a mixed bag prior to v1.11. Some drivers implemented this mechanism, some implemented it differently, some only worked on 1 "special" bus. As of v1.11 it should all be handled consistently now (+/- minor bugs per board).

I see, that sounds much better. I have mostly used the ll40ls driver, so I didn't realize other drivers acted differently in v1.10.


I was certain that the -b option would expect the same numbering as printed here (I'm clearly mistaken).

What if we extend the output to say on I2C bus 4 (1. external bus)?

I think this might be confusing for other people as well. Maybe the ll40ls help output can be modified, or this made clearer in some other way.

What do you suggest?

We have

  • internal as in used with -I, and
  • external as "4" in ll40ls #0 on I2C bus 4 (external), and
  • external as "1" in -X -b 1

Do we have terminology to differentiate the last two?

One possible solution is to print an explanation if the user tries to ll40ls start -X -b <num> with an invalid bus number. Something like

No ll40ls sensor found on bus <num>. Buses specified with `-b` are numbered based on FCU port names, such that port I2C A is bus 1, and so on.

Or a less verbose variant. I'm not sure if all FCUs name their ports "I2C A" etc. so that might need adjusting.

A drawback is that if the numbering using with -b is say, 1, 2, and the numbering in on I2C bus x is 2, 1, you'll never get the warning, but the sensors will be swapped.

@bkueng
Copy link
Member

bkueng commented Apr 23, 2020

Yes that is generally too verbose and would clutter the boot output unnecessarily.

@mortenfyhn
Copy link
Contributor Author

Yeah, I guess so.

Then I suggest a slight modification to your suggestion: on I2C bus 4 (corresponds to -b 1). I just think explicitly mentioning that this number is used with -b is useful.

@bkueng
Copy link
Member

bkueng commented Apr 23, 2020

Ok, I'll do the change.

@mortenfyhn
Copy link
Contributor Author

Great! Thank you.

@bkueng
Copy link
Member

bkueng commented Apr 24, 2020

See #14748

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Documentation 📑 Anything improving the documentation of the code / ecosystem
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants