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

APM Planner not working with mavrouter #1270

Closed
kaangoksal opened this issue Dec 22, 2022 · 25 comments
Closed

APM Planner not working with mavrouter #1270

kaangoksal opened this issue Dec 22, 2022 · 25 comments

Comments

@kaangoksal
Copy link

kaangoksal commented Dec 22, 2022

Hi,

I'm testing APM planner with mavrouter, and its currently not working, there are two major problems

  1. APM Planner sends a packet with id 2 to Ardupilot with the sys/comp id 1/1, this is wrong. it should use its own sys(252)/compid

mavlink-router/mavlink-router#382 (comment)

  1. Mission sync failure due to wrong target component id.

its mainly due that APM Planner sends messages to 1/190 sys/comp id, ardupilot does not advertise 1/190, so router doesn't route it. Details on how mavlinkrouter works is here: mavlink-router/mavlink-router#371

APM Planner needs to send messages to 1/1 not 1/190

I believe this is the line which is wrong.

current_partner_compid = MAV_COMP_ID_MISSIONPLANNER;

we need to be targeting uas comp id.

@kaangoksal
Copy link
Author

I added debug statements to mavlink-router yesterday night,

void Endpoint::_add_sys_comp_id(uint8_t sysid, uint8_t compid)
{
   uint16_t sys_comp_id = ((uint16_t)sysid << 8) | compid;

   if (has_sys_comp_id(sys_comp_id)) {
       return;
   }

   if ((sniffer_sysid != 0) && ((sys_comp_id >> 8) == sniffer_sysid)) {
       log_info("Sniffer sysid %u identified. [%d] is now sniffing all messages",
                sniffer_sysid,
                fd);
   }
   log_info("%s Adding sysid: %d compid: %d", _name.c_str() ,sysid, compid);
   _sys_comp_ids.push_back(sys_comp_id);

   // add to grouped endpoints as well
   for (auto e : _group_members) {
       e->_add_sys_comp_id(sysid, compid);
   }
}

APM planner sends message as 1/1 (which is the ardupilot) to mavlink-router

mavlink-router version v3+
Opened UART [4]usbUART: /dev/ttyACM0
UART [4]usbUART: speed = 115200
Opened UDP Server [5]MainUdp: 0.0.0.0:14550
Opened TCP Server [6] [::]:5760
usbUART Adding sysid: 1 compid: 1
TCP [8]dynamic: Connection accepted
dynamic Adding sysid: 252 compid: 1
dynamic Adding sysid: 1 compid: 1
TCP [8]dynamic: Connection closed

this throws off the mavlink router.

Endpoint [8]dynamic: got message 2 to -1/-1 from 1/1
	Known components:
		252/1
Endpoint [8] accepted message 2 to -1/-1 from 1/1
TCP [8]dynamic: Wrote 23 bytes
> TCP [8]dynamic: Got 26 bytes
dynamic Adding sysid: 1 compid: 1
Endpoint [4]usbUART: got message 111 to -1/-1 from 1/1
	Known components:
		1/1
Endpoint [5]MainUdp: got message 111 to -1/-1 from 1/1
	Known components:
Endpoint [5] accepted message 111 to -1/-1 from 1/1
UDP MainUdp: No one ever connected to us. No one to write for
Endpoint [8]dynamic: got message 111 to -1/-1 from 1/1
	Known components:
		252/1
		1/1

the issue is the message id 2,

@kaangoksal
Copy link
Author

kaangoksal commented Dec 23, 2022

Ok, I believe this is wrong

current_partner_compid = MAV_COMP_ID_MISSIONPLANNER;

It should record the component id that the drone is sending and use that, because of this mission exchange fails.

@AndKe
Copy link
Contributor

AndKe commented Feb 2, 2023

I tried component ID 1 - which is what the AP was using, with no luck - did you ever try this solution?

@kaangoksal
Copy link
Author

Yes, i forked the mavrouter for this. I made a new flag called ignore comp id. So now all messages for sys id goes to the same interface, ignores component id.

Here

https://github.com/kaangoksal/mavlink-router/tree/ignore_comp_id_fix

@kaangoksal
Copy link
Author

Let me know if you want more help with setting my new flag up

@AndKe
Copy link
Contributor

AndKe commented Feb 2, 2023

@kaangoksal thanks - as you have this running - I assume you have the current master on APMPlanner2 too.
I am trying to fix AP2. If you could print a debug text identifying the incorrect packets from AP2 .. that would help me a lot. - could you please do so? (I hope you have most of it accessible, and it's merely about adding some print functions)
Identifying the bad packets from AP2 would be of great help - I tried to fix them "blindly" and it did not work.

@kaangoksal
Copy link
Author

kaangoksal commented Feb 2, 2023 via email

@AndKe
Copy link
Contributor

AndKe commented Feb 4, 2023

also: please test the build mentioned here #1248 (comment) (latest appimage).
The build proves to me that my inability to fetch the waypoints was linked to my Ubuntu builds, not AP2 bug.

@kaangoksal
Copy link
Author

kaangoksal commented Feb 6, 2023

Setup

[UartEndpoint usbUART]
Device = /dev/ttyACM0 
Baud = 115200
MavlinkDialect = auto
#IgnoreCompId = true

[General]
TcpServerPort = 5760

[UdpEndpoint DroneBound]
mode = Normal
Address = 127.0.0.1
Port = 14550
#AllowSrcSysIn = 252

There are two issues, one is for some reason apm planner sends a message with sys/comp id 1/1, this makes the mavlink router think that it the system 1/1 also exists on the interface that the apm planner is attached.

[dynamic] Message 111 to -1/-1 from 1/1 will cause a routing change
dynamic Adding sysid: 1 compid: 1
Endpoint [4]usbUART: got message 111 to -1/-1 from 1/1
	Known components:
		1/1
Endpoint [4] rejected message 111 to -1/-1 from 1/1
Endpoint [7]dynamic: got message 111 to -1/-1 from 1/1
	Known components:
		252/1
		1/1

this can be mitigated with this to continue testing:

[UartEndpoint usbUART]
Device = /dev/ttyACM0 
Baud = 115200
MavlinkDialect = auto
#IgnoreCompId = true

[General]
TcpServerPort = 5760

[UdpEndpoint DroneBound]
mode = Normal
Address = 127.0.0.1
Port = 14550
AllowSrcSysIn = 252 #this is enabled now

@billbonney
Copy link
Member

It's not wrong. The original spec had the mission plan sent to component 190. That was how it worked.

TBH. The 'component id' just confuses the protocol. You really just need to send the message to the sys_id and let the system figure out where the message goes. ie. comp_id of 1 should be default.

I'm not sure about the 1/1 message. That maybe a bug. normally it would have just been ignored, but the the router just gets confused!

@kaangoksal
Copy link
Author

kaangoksal commented Feb 6, 2023

@billbonney then we need to fix the mavlink router, or add my new addition IgnoreCompId. Just opened a PR

mavlink-router/mavlink-router#385

we need to solve the 1/1 message for apm planner because that one is pretty bad.

@kaangoksal
Copy link
Author

kaangoksal commented Feb 6, 2023

@AndKe

https://github.com/kaangoksal/mavlink-router/tree/additional_debugging_for_apm

pull it and run the commands on the readme,

meson setup build . # you need this once

ninja -C build #builds it

here I added additional debug statements. It adds messages like

[APM Planner] Message 111 to -1/-1 from 1/1 will cause a routing change

we should not be seeing a message coming from the interface of the GCS as from 1/1, it should always be 2**/*

I used the latest app image release with all of my trials APM-Planner_2-2.0.30-rc1-x86_64.AppImage

@billbonney
Copy link
Member

APM Planner is connected to sys_id 1 comp_id 1. That is the drone on single drone system. APM Planner is 252 sys_id. I would expect to see packets from APM Planner to drone and vice versa.

I've not looked at mavrouter, so I can help there.

@kaangoksal
Copy link
Author

I already opened the pr for mavlink router. The problem with APM Planner is, it sends a packet with origin 1/1. It should never do that.

@billbonney
Copy link
Member

billbonney commented Feb 6, 2023

I'll put my neck on the the line and guess this is the bug send msg 1/1 @AndKe https://github.com/ArduPilot/apm_planner/blame/7897793b782180fea87fd6fd15d6cd4ee94631b1/src/uas/UAS.cc#L1431

@kaangoksal
Copy link
Author

I'll put my neck on the the line and guess this is the bug send msg 1/1 @AndKe https://github.com/ArduPilot/apm_planner/blame/7897793b782180fea87fd6fd15d6cd4ee94631b1/src/uas/UAS.cc#L1431

the system that I'm testing with does not have any gps attached

@billbonney
Copy link
Member

Nothing to do with GPS

https://mavlink.io/en/messages/common.html#TIMESYNC

@billbonney
Copy link
Member

Hi @kaangoksal,

the _encode message functions take the sending system_id and the sending component_id

The problem with the message when we get a MAVLINK_MSG_ID_TIMESYNC ie message id 111 in your log. Is that a response is sent, but the sending id/ comp id is set to that of the received message. ie 1/1 not 252/1 or 190

ie https://github.com/ArduPilot/apm_planner/blame/7897793b782180fea87fd6fd15d6cd4ee94631b1/src/uas/UAS.cc#L1431
is wrong

it should be
mavlink_msg_timesync_encode(systemId, componentId, &answer, &timeSync);

The sending of a message needs to follow this pattern

see https://github.com/ArduPilot/apm_planner/blame/7897793b782180fea87fd6fd15d6cd4ee94631b1/src/uas/UAS.cc#L2314

@kaangoksal
Copy link
Author

Hi @kaangoksal,

the _encode message functions take the sending system_id and the sending component_id

The problem with the message when we get a MAVLINK_MSG_ID_TIMESYNC ie message id 111 in your log. Is that a response is sent, but the sending id/ comp id is set to that of the received message. ie 1/1 not 252/1 or 190

ie https://github.com/ArduPilot/apm_planner/blame/7897793b782180fea87fd6fd15d6cd4ee94631b1/src/uas/UAS.cc#L1431 is wrong

it should be mavlink_msg_timesync_encode(systemId, componentId, &answer, &timeSync);

The sending of a message needs to follow this pattern

see https://github.com/ArduPilot/apm_planner/blame/7897793b782180fea87fd6fd15d6cd4ee94631b1/src/uas/UAS.cc#L2314

so we found the problem! I will work with the mavlink-router team to get that comp id issue merged, so we can have apm planner working with the router as well.

@billbonney
Copy link
Member

It's not a Mavlink router issue. It's a badly formatted message from apm planner. @AndKe @Arne-W will understand.

@kaangoksal
Copy link
Author

It's not a Mavlink router issue. It's a badly formatted message from apm planner. @AndKe @Arne-W will understand.

even though you fix this one (sending the time sync with wrong comp/sysid), mavlink-router needs to ignore comp ids to make mission sync work. At the moment mission sync does not work with APM Planner, but works with Mission planner.

Mission planner sends the missions to 1/1.
Apm Planner sends it to 1/190 (you can fix this by sending the mission requests to 1/1 on apm planner code)

There are two issues.

@AndKe
Copy link
Contributor

AndKe commented Feb 6, 2023

I totally agree with @billbonney on the guilty mavlink_msg_timesync_encode() - that's the only place where AP2 assembles TIMESYNC packets.

To my best knowledge - I also agree with @kaangoksal as I've been working with several self-made systems - and while sending to comp_id 190 is documented https://mavlink.io/en/messages/common.html#MAV_COMP_ID_MISSIONPLANNER - but still kind of odd - I can't remember to have seen such adressing before - and it is a trivial test to test for them with the other GCS's.

  • I suspect this CompID is reserved for mission planner, but in no way necessary. After all: missions can be exchanged with other comp_ID's too.
    All the other autopilot-GCS communication apparently works fine without using 190.

Finally: I would have built-and-tested more of this - but right now, my build-environment is no good and produces a AP2 that cannot exchange missions at all: #1248 (comment)

@billbonney
Copy link
Member

billbonney commented Feb 6, 2023

I suspect this CompID is reserved for mission planner, but in no way necessary. After all: missions can be exchanged with other comp_ID's too.
All the other autopilot-GCS communication apparently works fine without using 190.

I think you should add an option to change the comp_id to 1 (or change it be default) but allow it to be set to 190 (as that will work with Ardupilot Mega boards and older FW)

@Arne-W
Copy link
Contributor

Arne-W commented Feb 6, 2023

@billbonney Thank you for finding the culprit. I will fix the timesync message in the next days.
Regarding the comp_id there is already an option in the advanced Apm Planner config to change the Mavlink ID of the AP2. Perhaps we should add another field for changing the comp_id??

@billbonney
Copy link
Member

Perhaps we should add another field for changing the comp_id??

It's actually the comp_id in this file that needs to be allowed to change when writing missions and all.

int UASWaypointManager::setCurrentWaypoint(quint16 seq)

Like I said, it was always an off choice. Should just let the system decide on where to route messages.

Anyway, thanks @kaangoksal for bringing the issue up. Looks like we understand the root cause and can fix. Probs good to hit up the QGC guys to this issue to fix as well. 👍

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

No branches or pull requests

4 participants