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.

Sign up for GitHub

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

RADIO_STATUS message filtering on Solo #470

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from
Open

RADIO_STATUS message filtering on Solo #470

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 28 additions & 1 deletion ...n/java/org/droidplanner/services/android/impl/core/drone/autopilot/apm/solo/ArduSolo.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
import android.view.Surface;

import com.MAVLink.Messages.MAVLinkMessage;
import com.MAVLink.common.msg_radio_status;
import com.MAVLink.common.msg_statustext;
import com.MAVLink.enums.MAV_TYPE;
import com.o3dr.android.client.apis.CapabilityApi;
Expand Down Expand Up @@ -172,6 +173,26 @@ public SoloComp getSoloComp() {
return soloComp;
}

@Override
public void onMavLinkMessageReceived(MAVLinkMessage message) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The message id is correct, it's the system id and component id are set to a fixed value.

The fix is https://github.com/dronekit/dronekit-android/blob/develop/ClientLib/src/main/java/org/droidplanner/services/android/impl/core/drone/autopilot/apm/ArduPilot.java#L388

and

https://github.com/dronekit/dronekit-android/blob/develop/ClientLib/src/main/java/org/droidplanner/services/android/impl/core/drone/autopilot/generic/GenericMavLinkDrone.java#L581

The change here actually skips to message ID verification to allow the SiK message to be processed. Not sure why that isn't working on Solo. It work for ArduCopter/ArduPlane etc

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Later: You may need to look at this code for why the message is filtered https://github.com/dronekit/dronekit-android/blob/develop/ClientLib/src/main/java/org/droidplanner/services/android/impl/core/drone/autopilot/apm/ArduPilot.java#L394

DroneKit has way to many extends of classes, it gets really confusing at time 😜

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This doesn't fix the filtering for Solo. Solo gets msg_radio_status, not msg_radio. It's filtering the message because it's not included as a Mavlink exception message.

Copy link
Contributor Author

@kellyschrock kellyschrock Feb 20, 2017 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For this to work with Solo, need to add a separate case for MSG_RADIO_STATUS in ArduPilot (if you're disinclined to have the processing for that message in ArduSolo like I have it now), and implement the filtering exception in ArduSolo.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would be expecting RADIO_STATUS should be handled in GenericMavLinkDrone (not ArduPilot) as its a common MAVLink message https://github.com/dronekit/dronekit-android/blob/develop/ClientLib/src/main/java/org/droidplanner/services/android/impl/core/drone/autopilot/generic/GenericMavLinkDrone.java#L601

Either solution would work, but I think when I added the exception method the idea was that is should be used to override extra messages that you need to let through. (You may need to use processRadio... method to normalize the values)

The code for filter messages was adding as part of the plan to allow sys_id other than 1 to work.. That's the next step at least.


switch(message.msgid) {
// Evidently the sysid on RADIO/RADIO_STATUS messages on solo don't have their
// sysid set properly, so we handle it here without checking for that.
case msg_radio_status.MAVLINK_MSG_ID_RADIO_STATUS: {
msg_radio_status m_radio_status = (msg_radio_status) message;
processSignalUpdate(m_radio_status.rxerrors, m_radio_status.fixed, m_radio_status.rssi,
m_radio_status.remrssi, m_radio_status.txbuf, m_radio_status.noise, m_radio_status.remnoise);
break;
}

default: {
super.onMavLinkMessageReceived(message);
break;
}
}
}

/**
* No need to update the stream rates for Solo as it's being set by the companion computer
* @return
Expand Down Expand Up @@ -257,7 +278,11 @@ public void run() {
@Override
public void notifyDroneEvent(final DroneInterfaces.DroneEventsType event) {
switch (event) {
case HEARTBEAT_FIRST:
case HEARTBEAT_FIRST: {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we don't have a onMessage received overloaded in this object, does the 'double connect message' problem go away?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Turns out it does. I'll also investigate why I'm getting double disconnect messages as well.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, the problem goes deeper than that. Whatever changes happened here cause the Solo to not know it's connected. I'm connected, getting radio messages, telemetry, etc, but Drone.isConnected(), returns false without that extra HEARTBEAT_FIRST message processing in there.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Interesting. So much indirection with the use of extends can make the code hard to follow! It's would almost be better if the GenericMavLinkDrone object received all messages first and then call a potentially derived processOnMavLinkMessageReceived implemented in specializations to help with the synchronization issues we are seeing. (i.e. the checkIfFlying call is obviously not happening on first connection before anything else)

Timber.i("heartbeat_first");
break;
}

case CONNECTED:
Timber.i("Vehicle " + event.name().toLowerCase());
//Try connecting the companion computer
Expand Down Expand Up @@ -368,6 +393,8 @@ protected boolean isFeatureSupported(String featureId) {
@Override
protected void processSignalUpdate(int rxerrors, int fixed, short rssi, short remrssi, short txbuf,
short noise, short remnoise) {
Timber.d("processSignalUpdate(): rssi=" + rssi);

final double unsignedRemRssi = remrssi & 0xFF;

signal.setValid(true);
Expand Down