search

ArduPilot / ardupilot

ArduPlane, ArduCopter, ArduRover, ArduSub source
http://ardupilot.org/
GNU General Public License v3.0
10.23k stars 16.82k forks source link

ArduCopter-Heli: Initially reports HEARTBEAT.type=2 (MAV_TYPE_QUADROTOR) #11591

Closed DonLakeFlyer closed 2 years ago

DonLakeFlyer commented 5 years ago

This is happening for me on ArduHeli 3.6.9 ChibiOS build. I assume it will happen on other builds as well.

The first heartbeat you get after boot has HEARTBEAT.type=2 (MAV_TYPE_QUADROTOR). Subsequent heartbeats you get will come through as HEARTBEAT.type=4 (MAV_TYPE_HELICOPTER).

Since the first one if wrong this causes QGC to show the incorrect ui since it thinks it's a multi-rotor.

rmackay9 commented 5 years ago

@bnsgeyer, FYI if you haven't heard of this before. I would guess that we are sending heart beats before we've set the mav type so perhaps we need a flag stop us from sending heartbeats before that's done.

bnsgeyer commented 5 years ago

@rmackay9 I am not sure where to start on this. I’m guessing this would be somewhere in system.cpp. Need some help.

magicrub commented 5 years ago

Possible fix: https://github.com/ArduPilot/ardupilot/pull/11596

bnsgeyer commented 5 years ago

@rmackay9 is the solution that @magicrub proposed a viable solution? I really don’t have the expertise on this part of the code.

rmackay9 commented 5 years ago

@bnsgeyer, I think this could work.. Peter's brought up a potential issue but this might be OK.

peterbarker commented 5 years ago

On Mon, 17 Jun 2019, Randy Mackay wrote:

@bnsgeyer, I think this could work.. Peter's brought up a potential issue but this might be OK.

What was getting upset with the initial heartbeat being incorrect?

We'll have the same problem on Rover with boat vs land-vehicle...

rmackay9 commented 5 years ago

@peterbarker, QGC is upset because it uses the first heartbeat to decide what type of vehicle it is apparently.

DonLakeFlyer commented 5 years ago

QGC is upset because it uses the first heartbeat to decide what type of vehicle it is apparently.

Correct. QGC shows different parts of it's ui based on vehicle type. The vehicle type is set from the first heartbeat and is not expected to change after that.

peterbarker commented 5 years ago

@DonLakeFlyer If we were to send "generic" until we know what type the vehicle actually is, would that be enough to not break your interfaces? The problem in AP is that we may need to send heartbeats out before we know what the vehicle type actually is.

rmackay9 commented 5 years ago

@peterbarker, I wonder why we need to send heartbeats.. something about GCSs detecting mavlin1 vs mavlink2?

DonLakeFlyer commented 5 years ago

If we were to send "generic" until we know what type the vehicle actually is, would that be enough to not break your interfaces?

Yes it would. It would treat it as a mavlink generic vehicle. Which does not support vehicle setup.

magicrub commented 5 years ago

@donlakeflyer would you rather not get any heartbeats until we're ready to send the correct one? I think a wrong heartbeat is the wrong way to go. It only takes us less than another half second to know what kind of vehicle we are.

peterbarker commented 5 years ago

On Mon, 1 Jul 2019, Randy Mackay wrote:

@peterbarker, I wonder why we need to send heartbeats.. something about GCSs detecting mavlin1 vs mavlink2?

heartbeats make the traffic go 'round.

Without heartbeats mavlink routing doesn't work.

I believe it's in the spec you have to heartbeat.

We require heartbeats from our GCS to avoid failsafing - for no discernable reason.

MAVProxy requires heartbeats before it will start, ordinarily.

rmackay9 commented 5 years ago

@peterbarker, I should have been more clear. What I mean is why do we have to send heartbeats before the vehicle is initialized. Why not just wait the additional 3 seconds (or whatever) until we have loaded all params and know what type of vehicle we are?

DonLakeFlyer commented 5 years ago

would you rather not get any heartbeats until we're ready to send the correct one?

Yup

bnsgeyer commented 5 years ago

Sorry. Fat fingers.

rmackay9 commented 5 years ago

Ok great. I suspect TomP's fix above will work then. @magicrub do you want to PR that?

magicrub commented 5 years ago

Same fix for all vehicles?

rmackay9 commented 5 years ago

@magicrub, for Rover at least I think we should have a similar change. I'm unsure what Tridge and Peter know that we don't know so let's see the fix for Copter go in first..

peterbarker commented 5 years ago

On Tue, 2 Jul 2019, Randy Mackay wrote:

@peterbarker, I should have been more clear. What I mean is why do we have to send heartbeats before the vehicle is initialized. Why not just wait the additional 3 seconds (or whatever) until we have loaded all params and know what type of vehicle we are?

You could expect any traffic emitted before them to be dropped. That's kind of bad if you're sending out lots of boot-up information.

My chief concern was the apparent out-and-out breakage of the sensor-failure loop.

Now the thing here is that we've read parameters by the time we start doing the mavlink thing anyway! I've just dig into it and made what I think is a more correct fix:

https://github.com/ArduPilot/ardupilot/pull/11707

IamPete1 commented 2 years ago

closed by https://github.com/ArduPilot/ardupilot/pull/11707