tfmini fixes #8726

ChristophTobler · 2018-01-18T13:20:53Z

This fixes the tfmini for other boards than Pixhawk v2.

The main issue was the default device path https://github.com/PX4/Firmware/blob/master/src/drivers/distance_sensor/tfmini/tfmini.cpp#L84

The driver was started without any specific device path which resulted in problems for other boards -> e.g. disables other sensors.
Stopping the driver and restarting it on the correct device path didn't work... not sure why. By requiring the device path we can solve the issue.

mainly the default device path doesn't work

This makes sure the driver fails if the device path is invalid (::open fails)

dagar · 2018-01-18T16:18:54Z

src/drivers/distance_sensor/tfmini/tfmini.cpp

-// designated SERIAL4/5 on Pixhawk
-#define TFMINI_DEFAULT_PORT		"/dev/ttyS6"
+#define NAME "tfmini"
+#define DEVICE_PATH "/dev/" NAME


These are different things.
/dev/tfmini is the px4 device driver (created by this module).
/dev/ttyS6 or whatever needs to be the physical serial port.

I think this is the case, isn't it? It is not directly replacing it

Sorry you're right, I should really stop trying to view diffs on my phone. There's no default port then?

hehe, no worries :) Yes, there's no default port anymore. I think it makes no sense to have one if it's different on every board

What's wrong with defaulting to RANGE_FINDER0_DEVICE_PATH and letting the command line override?

dagar · 2018-01-18T16:20:03Z

ROMFS/px4fmu_common/init.d/rc.sensors

+	if ver hwcmp PX4FMU_V3
+	then
+		# start the driver on serial 4/5
+		tfmini start -d /dev/ttyS6


Can we do this more generically? There are way too many variants now to maintain these special cases.

I know... But I can't think of anything... On every board, it's different and we would need to link it to https://github.com/ChristophTobler/Firmware/blob/master/ROMFS/px4fmu_common/init.d/rcS#L641-L672 as most boards need to use this device path.

Fair enough, I'll see if I can come up with something in the near future. #8302

@ChristophTobler So based on this, is the documentation here accurate? It looks like the TFmini is only automatically started on Pixhawk FMUv3 (but not before or later) ... right?

If that is the case then should we not link to instructions for enabling on other boards - i.e. instructions on how to add a driver to a config and how to start/stop the driver and specify the port to watch?

@dagar

Will the changes you propose in generalize serial port configuration #8302 make enabling sensors consistent and generic? I'm envisaging that you would set SENS_EN_TFMINI with the value of the port you wanted to use ... and ideally that this setting would automatically disable any other thing that is trying to use the port.

Do you know what we need to do to create module docs (like this for tfmini? That would allow me to have somewhere to link to for "using the driver" instructions.

I wasn't sure how to structure it. Configuring a parameter per serial port is a little more straightforward, but it's less modular. Parameters to enable per device are relatively easy, and keeps it separate, but what happens if you configure a conflict? Who takes priority?

I believe all of that documentation is generated from the module itself and Tools/px_process_module_doc.py https://github.com/PX4/Firmware/blob/master/src/drivers/px4fmu/fmu.cpp#L3394

@dagar Plug'n'play would be nice :-). I understand your dilemma and there is no easy solution. My "docs" point of view is that it doesn't matter as long as we're completely consistent. Personally though I'd go for the less-modular per-serial port option because debugging conflicts would be a pain. Is there a way to reduce the modularity issues - I'm thinking using toolchain to build per-port mappings from driver directories (maintaining current lists).

With respect to docs, I believe @bkueng new module shows what would need to be added for this module docs: https://github.com/PX4/Firmware/blob/master/src/templates/module/module.cpp
Anything else need to be done Beat?

@ChristophTobler As above, please confirm the documentation is accurate and confirm the FMU versions this starts "out of the box" for.

@hamishwillee

It looks like the TFmini is only automatically started on Pixhawk FMUv3 (but not before or later) ... right?

correct. For other boards you need to manually disable mavlink on the port (rcS) and start the driver.

Cheers, I have added you as reviewer for this update in PX4/PX4-user_guide#138

ChristophTobler · 2018-01-18T16:42:32Z

src/drivers/distance_sensor/tfmini/tfmini.cpp

@@ -173,7 +174,7 @@ class TFMINI : public device::CDev
 extern "C" __EXPORT int tfmini_main(int argc, char *argv[]);

 TFMINI::TFMINI(const char *port, uint8_t rotation) :
-	CDev("TFMINI", RANGE_FINDER0_DEVICE_PATH),
+	CDev(NAME, DEVICE_PATH),


@dagar

What's wrong with defaulting to RANGE_FINDER0_DEVICE_PATH and letting the command line override?

I guess you're talking about this?

Can we have several range finders this way?

dagar · 2018-01-19T05:01:32Z

Looking good, but why drop the default port? Other boards (like aero) can specify something else.

ChristophTobler · 2018-01-19T07:38:52Z

@dagar I dropped it because it creates more confusion/damage, I think.
By requiring the port, people actually need to check which port to use (and maybe disable mavlink). Also, having a default port when it's different on every board seems weird.
But I agree, there needs to be a solution where the user can just enable it with a parameter. But for that we currently would need to add a bunch of if cases in rcS and rc.sensors... (which I could do)

this is more consistant and should enable it on qurt/linux

LorenzMeier · 2018-01-26T23:41:12Z

Could you check Circle / rebase? Thanks!

ChristophTobler · 2018-01-27T12:30:26Z

@LorenzMeier done

hamishwillee · 2018-01-31T00:22:41Z

ROMFS/px4fmu_common/init.d/rc.sensors

@@ -396,7 +396,11 @@ fi
 # Benewake TFMini
 if param greater SENS_EN_TFMINI 0
 then
-	tfmini start
+	if ver hwcmp PX4FMU_V3


Why is this specific to this board? Previously we enabled this on every board - since they have to set SENS_EN_TFMINI for this to be started, what difference does it make?

The difference is the device (-d /dev/ttyS6). Previously, It could happen that it breaks other devices/sensors...

weiminshen99 · 2018-05-18T22:11:44Z

Hi, All,
For Pixhawk_Mini, which device path should I use for TFMINI?
I have tried all /dev/ttyS0 thru ttyS6 with no luck.
Thanks for your assistance and I need it badly for my application.

weiminshen99 · 2018-05-18T23:35:49Z

To follow up, I connected tfmini to the telem port of the pixhawk-mini, and enabled the tfmini through QGC, then I used the Tools/mavlink_shell.py to check the status and test:
% tfmini status
It was connected to /dev/ttyS6, but cannot read anything. I then tried other ports:
% tfmini stop
% tfmini start -d /dev/ttySn # where n=0,...,6
% tfmini status
For each port, I checked the status and run a test, but none worked.

For your information, the same procedure worked on a regular pixhawk on serial4/5 port. Using the shell, I can see that tfmini was connected to /dev/ttyS6 and “tfmini test” shows it is working.

hamishwillee · 2018-05-21T01:07:20Z

@weiminshen99 Generally speaking not a good idea to piggy-back onto closed issues. The right places for discussing issues are here: https://docs.px4.io/en/#support

According to the docs https://docs.px4.io/en/sensor/rangefinders.html#tfmini serial 4/5 is the correct port and you need to set SENS_EN_TFMINI > 0. On this board you should not need to install the mini in firmware or start tfmini yourself.

weiminshen99 · 2018-05-21T05:38:08Z

Hi, Hamish, Thanks for your reply and I appreciate your suggestions. Please notice that there is no serial 4/5 on this Pixhawk_Mini board, and that was why I asked the question. On a regular Pixhawk (not mini) boards, I have been using TFMINI successfully based on the document you mentioned. Do you have any suggestions where I can find more information about this particular question? Thanks!

…

On May 20, 2018, at 6:07 PM, Hamish Willee ***@***.***> wrote: @weiminshen99 <https://github.com/weiminshen99> Generally speaking not a good idea to piggy-back onto closed issues. The right places for discussing issues are here: https://docs.px4.io/en/#support <https://docs.px4.io/en/#support> According to the docs https://docs.px4.io/en/sensor/rangefinders.html#tfmini <https://docs.px4.io/en/sensor/rangefinders.html#tfmini> serial 4/5 is the correct port and you need to set SENS_EN_TFMINI > 0. On this board you should not need to install the mini in firmware or start tfmini yourself. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#8726 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AXqet39hNip9hLb1uEZ77-3Jqz2_Z3Evks5t0hNVgaJpZM4Ri6Og>.

hamishwillee · 2018-05-21T05:53:02Z

@weiminshen99 OK, it may be that the docs are wrong. If you look at current master here it looks you must also set SYS_COMPANION 0 and then start tfmini on /dev/ttyS2 - TELEM2 usually.

Let me know if that works.

weiminshen99 · 2018-05-23T14:34:47Z

Thanks! I will test and let you know.

…

Sent from my iPad

On May 20, 2018, at 10:53 PM, Hamish Willee ***@***.***> wrote: @weiminshen99 OK, it may be that the docs are wrong. If you look at current master here it looks you must also set SYS_COMPANION 0 and then start tfmini on /dev/ttyS2 - TELEM2 usually. Let me know if that works. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.

ChristophTobler requested a review from dagar January 18, 2018 13:20

ChristophTobler mentioned this pull request Jan 18, 2018

tfmini not working on V4 hardware #8713

Closed

LorenzMeier previously approved these changes Jan 18, 2018

View reviewed changes

ChristophTobler dismissed LorenzMeier’s stale review via 797d9a1 January 18, 2018 14:11

ChristophTobler added 3 commits January 18, 2018 17:02

tfmini: fixes for other boards than FMUv3

7c9dcb5

mainly the default device path doesn't work

rc.sensors: start tfmini with appropriate device path for FMUv3

94fa175

tfmini: move part from constructor to init()

ed69e63

This makes sure the driver fails if the device path is invalid (::open fails)

ChristophTobler force-pushed the pr-tfmini_fixes branch from 797d9a1 to ed69e63 Compare January 18, 2018 16:04

dagar reviewed Jan 18, 2018

View reviewed changes

ChristophTobler commented Jan 18, 2018

View reviewed changes

tfmini: use px4_open/read and default range finder device path

970ef66

this is more consistant and should enable it on qurt/linux

ChristophTobler force-pushed the pr-tfmini_fixes branch from 584902b to 970ef66 Compare January 19, 2018 15:33

LorenzMeier approved these changes Jan 27, 2018

View reviewed changes

LorenzMeier merged commit 92dbb16 into PX4:master Jan 27, 2018

ChristophTobler deleted the pr-tfmini_fixes branch January 29, 2018 06:59

hamishwillee reviewed Jan 31, 2018

View reviewed changes

hamishwillee mentioned this pull request May 23, 2018

Pixhawk mini does not have serial 4/5 so docs for TFmini wrong PX4/PX4-user_guide#239

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

tfmini fixes #8726

tfmini fixes #8726

ChristophTobler commented Jan 18, 2018

dagar Jan 18, 2018

ChristophTobler Jan 18, 2018

dagar Jan 18, 2018

ChristophTobler Jan 18, 2018

dagar Jan 18, 2018

dagar Jan 18, 2018

ChristophTobler Jan 18, 2018 •

edited

Loading

dagar Jan 18, 2018

hamishwillee Jan 28, 2018

dagar Jan 29, 2018

hamishwillee Jan 30, 2018

ChristophTobler Jan 30, 2018

hamishwillee Jan 31, 2018

ChristophTobler Jan 18, 2018

dagar commented Jan 19, 2018

ChristophTobler commented Jan 19, 2018 •

edited

Loading

LorenzMeier commented Jan 26, 2018

ChristophTobler commented Jan 27, 2018

hamishwillee Jan 31, 2018

ChristophTobler Jan 31, 2018

weiminshen99 commented May 18, 2018

weiminshen99 commented May 18, 2018

hamishwillee commented May 21, 2018

weiminshen99 commented May 21, 2018 via email

hamishwillee commented May 21, 2018

weiminshen99 commented May 23, 2018 via email

tfmini fixes #8726

tfmini fixes #8726

Conversation

ChristophTobler commented Jan 18, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ChristophTobler Jan 18, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dagar commented Jan 19, 2018

ChristophTobler commented Jan 19, 2018 • edited Loading

LorenzMeier commented Jan 26, 2018

ChristophTobler commented Jan 27, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

weiminshen99 commented May 18, 2018

weiminshen99 commented May 18, 2018

hamishwillee commented May 21, 2018

weiminshen99 commented May 21, 2018 via email

hamishwillee commented May 21, 2018

weiminshen99 commented May 23, 2018 via email

ChristophTobler Jan 18, 2018 •

edited

Loading

ChristophTobler commented Jan 19, 2018 •

edited

Loading