UAVTalk: Need to accept instance IDs on single-inst objects because of LP impl drift

[humanoid2050]

• Seen in version (release version or githash): all
• On flight controller (if applicable):

Issue details: This code base has been forked from other similar code bases. However, the uavtalk protocol here is not the same as some of the others (i.e. Librepilot https://librepilot.atlassian.net/wiki/spaces/LPDOC/pages/8552471/UAVTalk). Since this code base is under actual development, I suggest incrementing the protocol version to 3 to indicate that it is no longer compatible with other versions.

[tracernz]

See previous conversation on this issue https://github.com/d-ronin/dRonin/pull/1741#discussion_r114068747

[humanoid2050]

All fair points. As it happens, trying to write code compatible with both dRonin and LibrePilot implementations of Uavtalk "version 2" cannot be done in a robust fashion. I can tell my code which to use at runtime, but that means it's up to a human to remember. At the end of the day, do what makes sense for your code base, this is just my 2 cents.

[tracernz]

Thanks for the report. I had worried that might be the case.

Out of curiosity, can you reveal what you are working on?

[humanoid2050]

It's still sort of vague and only in my head, but the end goal is going to have a ROS node (light weight, can run on a raspberry pi zero) that connects directly to a cc3d-family flight controller (I've already got the usb-hid interface working) and controls the quadcopter using uavtalk. To make it accessible, I'm looking to support dRonin since it's under active development and librepilot since it's available in the Ubuntu repos. I'm probably going to have the ROS node expose a mavlink interface to make control from an external autopilot possible. I expect to pop it up on my GitHub when it's further along.

[tracernz]

Sounds fun! @peabody124 (of Tau Labs) has also talked about ROS support.

dRonin also runs directly on a full-size Raspberry Pi with the FlyingPIO hat (https://github.com/d-ronin/dronin-hw/blob/master/flyingpio/revB/mfg_files/flyingpi.PDF) for sensors/IO. Your ROS layer would likely work well there too.

[peabody124]

@humanoid2050 @tracernz yeah i ended up writing a ROS interface back in the day. i definitely wrote a full node that ran on the flight controller and then spoke over serial (whatever the ros serial transport layer is). i also wrote some code for the desktop that took the packets from the FSK telemetry code and then created various ROS properties. it was useful for visualization of data (e.g. https://www.instagram.com/p/BCFwLMGlquZ/ https://www.instagram.com/p/BCKKdLWFqlN/ https://www.instagram.com/p/BCJYDuglqrb/ https://www.instagram.com/p/BCRgrnjFquF/)

it's been ages since I looked at it so I don't recall terribly well. i found that ROS didn't have a lot of common used values that were appropriate for arial values.

this was all on the @TauLabs codebase.

[humanoid2050]

@tracernz @peabody124 After some thought, maybe the real question I should be asking is whether or not the STM32F4 family of flight controllers can support a full Mavlink interface. Is there enough computational and memory overhead available for this?

I put together a simple Uavtalk/ROS bridge already, and something about having ~100 publishers and ~100 subscribers on a single node didn't seem that useful. Also, it doesn't really do anything to help a user express "please altitude hold at this lat/lon" in a form compatible with the internal Uavobject structure.

[peabody124]

Yeah if you want to give ROS the ability to control there are a lot of things that have to happen, and it's a lot more detailed than just the protocol layer. The approach I would favor is some degree of authoritative control on the flight control that can toggle back to a transmitter for safety (hence why I went to writing a full ROS node on the F4). However, yeah, it seemed easy to go into an alphabet soup of ROS nodes and I never found a good pseudo-authorative list of what nodes should be supported for, say navigation, and how they should be represented to leverage existing ROS control systems.

[mlyle]

Hey all--- I am travelling and mostly out of touch, but a few thoughts.

First, what's the biggest interop difference between LP and DR? I tried to make it so that the consistently defined portions of the protocol would stay the same with the past impl but must have screwed up somewhere. I would like to keep interop if possible.

Second, I don't like Mavlink so much and really like uavtalk--- I like that the "real" internal state is exposed and can be externally driven. But I agree the present object and task structure doesn't make it easy. I will give some thought on how best to do this without creating another layer of "desired" objects.

Right now it is possible to set Stabilization desired flight read only and directly drive the Stabilization loop. It is also possible to set a waypoint location and select that location.

This product cycle I am planning on refactoring the Stabilization loop to include altitude control and separating the inner and outer loop modes. It may be possible to positively control the path follower modes as part of this and in so doing simplify external mode control.

On Tue, Feb 20, 2018, 9:12 AM peabody124 notifications@github.com wrote:

Yeah if you want to give ROS the ability to control there are a lot of things that have to happen, and it's a lot more detailed than just the protocol layer. The approach I would favor is some degree of authoritative control on the flight control that can toggle back to a transmitter for safety (hence why I went to writing a full ROS node on the F4). However, yeah, it seemed easy to go into an alphabet soup of ROS nodes and I never found a good pseudo-authorative list of what nodes should be supported for, say navigation, and how they should be represented to leverage existing ROS control systems.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/d-ronin/dRonin/issues/2076#issuecomment-366972954, or mute the thread https://github.com/notifications/unsubscribe-auth/AAFjFxRFN-9tO55zbEb5ar5U6i6VisUHks5tWsTIgaJpZM4SLRPR .

[humanoid2050]

The big difference is that LP requires an instance id for all objects, set to 0 if it's a single instance object. The field is optional in DR (TL too). The net result is that single instance objects are 2 bytes too big/small when you go between uavtalk implementations.

I definitely agree that there is power in exposing and controlling the FC internal state the way uavtalk does. My thing is that it's not terribly compatible with a high level mission planning and execution system. That's why I think there is value in being able to accept a whole stack of mavlink commands, and execute them by manipulating the internal uavobjects via uavtalk.

[mlyle]

@tracernz-- turns out LP diverged from the original and not us ;)

IMO it would be a mistake to bump the version number to represent an earlier version of the protocol.

I will work on tolerating in the receive path in the flight side zero instids on single instance objects. We will still not send them because it's a density difference (and the length field can be used to discriminate).

@humanoid2050 --- I will also work on making it easier to do control tasks from uavtalk--- it has been on my to-do list for a long time. Part of the reason I don't want to embrace Mavlink is I'd like the ability to specify higher level goals than just positions and segments and have the controller do the work (and thus move higher level controllers further and further from real-time and actual machine performance).

On Tue, Feb 20, 2018, 1:01 PM humanoid2050 notifications@github.com wrote:

The big difference is that LP requires an instance id for all objects, set to 0 if it's a single instance object. The field is optional in DR (TL too). The net result is that single instance objects are 2 bytes too big/small when you go between uavtalk implementations.

I definitely agree that there is power in exposing and controlling the FC internal state the way uavtalk does. My thing is that it's not terribly compatible with a high level mission planning and execution system. That's why I think there is value in being able to accept a whole stack of mavlink commands, and execute them by manipulating the internal uavobjects via uavtalk.

— You are receiving this because you commented.

Reply to this email directly, view it on GitHub https://github.com/d-ronin/dRonin/issues/2076#issuecomment-367044978, or mute the thread https://github.com/notifications/unsubscribe-auth/AAFjFxydjl-jzrBAmjLzu4nXp1wSGFfGks5tWvpSgaJpZM4SLRPR .

[tracernz]

@mlyle Ahhh, that is really annoying :disappointed: https://github.com/librepilot/LibrePilot/commit/2f0974fda942f28b09b866991121aef9cb939cce#diff-77fe0b567b7f0352a55dc1da63892a73

[humanoid2050]

If LP changed, I'm happy to close this as "not our problem".

@mlyle I'd be interested to know more of your thoughts on the subject of mission control as it relates to uavtalk. Probably for now I will be making a ROS node on an embedded computer that speaks uavtalk on one side, mavlink on the other, and does translation/control operations in the middle.

[mlyle]

Right now the actual control path is a bit of a mess. There's ManualControl which mostly does nothing; the ManualControl task either drives StabilizationDesired, AltitudeHoldDesired, LoiterDesired, or just the flight mode and lets other tasks drive stabdesired. The actual inner Stabilization modes sometimes do stick scaling themselves, or sometimes ManualControl does it first.

The actual architecture of controllers is pretty good, and they're fairly logically broken into tasks (though some of this dates to resource limitations that no longer affect us). But the resulting dataflow through objects is a mess, and complicates doing anything fancier and also makes driving uavos externally difficult.

I can't look at code from here, but the jist of my previous thoughts on the topic were:

There's some real logical things to do immediately --- StabilizationDesired should have a thrust axis and do vtol altitude hold. The vtol path follower should use the core Stabilization thrust loop. StabilizationDesired should include units and the stab task should do all the stick scaling when necessary. And an appropriate interface should be added to drive stabdesired with that is appropriately safe and simple to use.

This would mean that most external controllers would just need to drive stabdesired and/or pathdesired, and pay attention to any objects it cares about (positionactual, attitudeactual, velocityactual.... Or if it doesn't like our estimator it could look directly at the sensor objects, etc)....

It also opens the door to have a higher order object specifying multiple goals and associated costs... And letting the controller search for good solutions with contradictory goals (try to never pitch backwards, control yaw rate below 60 deg/s, try to stay close to this spline).

On Tue, Feb 20, 2018, 2:24 PM humanoid2050 notifications@github.com wrote:

If LP changed, I'm happy to close this as "not our problem".

@mlyle https://github.com/mlyle I'd be interested to know more of your thoughts on the subject of mission control as it relates to uavtalk. Probably for now I will be making a ROS node on an embedded computer that speaks uavtalk on one side, mavlink on the other, and does translation/control operations in the middle.

— You are receiving this because you were mentioned.

Reply to this email directly, view it on GitHub https://github.com/d-ronin/dRonin/issues/2076#issuecomment-367070988, or mute the thread https://github.com/notifications/unsubscribe-auth/AAFjF6PZUkSpz15y_Kp4Pl-u0-2jV2fhks5tWw3CgaJpZM4SLRPR .

[humanoid2050]

Hmmm... I think we may have a similar picture in mind. The high level stuff is managed externally from the FC. Control is executed by manipulating the Uavobject states to pick the appropriate stabilization mode and set the relevant parameters. Advanced things like waiting for some event or state is achieved by watching objects containing the relevant information, at which point the external mission manager can increment its internal state. Does this sound about right?

[mlyle]

Yes, for now. But I'm hoping the controller can take higher level goals eventually--- stay within these bounds, search for a solution that optimizes these cost functions. Then mission planning is just about crafting these cost functions and flipping between them on very rare state changes.

What you describe is possible now but messy because of the complexity of the dataflow. I think simplifying that will really help and be a good first step before building the state-predicting and optimizing infrastructure I am hoping to develop.

On Tue, Feb 20, 2018, 10:38 PM humanoid2050 notifications@github.com wrote:

Hmmm... I think we may have a similar picture in mind. The high level stuff is managed externally from the FC. Control is executed by manipulating the Uavobject states to pick the appropriate stabilization mode and set the relevant parameters. Advanced things like waiting for some event or state is achieved by watching objects containing the relevant information, at which point the external mission manager can increment its internal state. Does this sound about right?

— You are receiving this because you were mentioned.

Reply to this email directly, view it on GitHub https://github.com/d-ronin/dRonin/issues/2076#issuecomment-367194777, or mute the thread https://github.com/notifications/unsubscribe-auth/AAFjF7efp3pEEdnfGt0VrNj8B6BtFUPwks5tW4GWgaJpZM4SLRPR .

[mlyle]

Reopening so I can look at feasibility of accepting 0 instid

[humanoid2050]

Sorry about that. I figured since this one wasn't the one that diverged, it shouldn't be the one that has to change.

[mlyle]

No worries. I think it's a good robustness change to accept 0 instids on single instance objects (and in accordance with the robustness principle--- be conservative in what you send, be liberal in what you accept)

The only cost is the risk of accepting an errant object--- right now we depend on uniqueness of a 31 bit identifier plus the length of the object, and now there will be two different lengths we accept, so a slightly higher risk of getting confused with mixed versions -- but still manageable)

On Sun, Feb 25, 2018, 8:13 AM humanoid2050 notifications@github.com wrote:

Sorry about that. I figured since this one wasn't the one that diverged, it shouldn't be the one that has to change.

— You are receiving this because you modified the open/close state.

Reply to this email directly, view it on GitHub https://github.com/d-ronin/dRonin/issues/2076#issuecomment-368308313, or mute the thread https://github.com/notifications/unsubscribe-auth/AAFjF3Htupqa8DzzF3t5WGoOpzSfHFmtks5tYVyKgaJpZM4SLRPR .

[mlyle]

Can you easily try #2082 --- it should receive frames for single-instance objects with instance ids (though we will never send them-- no inst len means instid 0).

[humanoid2050]

I was about to say I will test it, but then I started thinking.... To activate your proposed code path, that would require compiling a version of the LP GCS against the DR message set. Doable. However, I have a suspicion that there will be problems resulting from the differences in the GCSTelemetryObject between the two projects (there are some). Then there is the problem that from the perspective of the LP GCS, all the single instance messages will be 2 bytes too short. Net result... I'm not entirely sure about the practical use case for this change.

[mlyle]

I think the primary test would be whether your code can be made to easily work with both--- e.g. you doing a similar thing to cope with missing/present instance id; you adjusting to always send instance id yourself, and seeing whether you can talk.

[humanoid2050]

Derp. I got the new version compiled and installed, ran it against my existing (LP derived) code base and got bidirectional comms as indicated by the gcs and flight states transitioning to "connected". I still have to tweak a couple things on the receiving side of my code base, but the outbound side that always sends an instance ID seems to play well with the modified DR FC code.

[mlyle]

@humanoid2050 Awesome! Thanks for the report and the test.

[humanoid2050]

@mlyle just to keep you up to date, I went over to the librepilot group (they seem to do most of their work in a different website) and made a nuisance of myself. I think they will be incrementing their version number since they changed the protocol implementation. I will keep you posted.

[humanoid2050]

@mlyle @tracernz I had a thought. I'm proposing to the LP group that bit 4 of the message type byte be used to indicate the presence of the instanceID field in the uavtalk stream for uavtalk protocol version 3.

https://librepilot.atlassian.net/browse/LP-583

How does that sound to you?

[mlyle]

:/ why bump the version, though, if we had an acceptable workaround here?

[tracernz]

There are a few other implementations out there that would be broken by LP. Why have a version if you're not going to use it? :/ Admittedly it's probably too late on this case unfortunately.

[humanoid2050]

@mlyle I fear I may have gotten too clever for my own good. It's perfectly reasonable for DR to keep with its implementation of the original version 2 spec. I think my point was more along the lines of making a version that both ideologies (optional or mandatory instance ID) could use and the protocol would simply self-describe the contents.

@tracernz I think LP will increment their version. I've already made those changes to push to their code base. They need to increment whether or not they implement my clever use of bit 4 to indicate the presence of the instance id.

[mlyle]

@humanoid2050 I think the solution we have here works (cope with an instance id being missing on a single-inst object) and would head towards interop... bumping versions will prevent wrong-versions of GCS talking without a whole lot of workaround, etc.

I would rather LP did nothing. Why exactly did you ask them to bump the version--- doesn't the solution here for interop work?

[humanoid2050]

I talked to LP because at the end of the day, they changed the protocol and didn't increment the version. That's just bad configuration management. Your solution of interoperation allows you to receive LP patterned uavtalk, but they still can't receive what DR sends back.