iNavFlight / inav

INAV: Navigation-enabled flight control software
https://inavflight.github.io
GNU General Public License v3.0
3.17k stars 1.48k forks source link

Feature request: Sharing UART between SerialRX and uni-directional telemetry #1411

Closed rodizio1 closed 6 years ago

rodizio1 commented 7 years ago

Right now, I'm running a setup with Cleanflight and a Raspberry Pi that receives LTM telemetry from the FC and also sends MSP R/C commands to the FC. On the Pi, I use the same serialport for this. In Cleanflight, I have to use two different UARTs.

Now I would like to use iNav with a SPRacing F3 FC and a GPS. UART1 is already used for the USB port, leaving only two UARTs for telemetry, R/C and GPS. If I could run telemetry and R/C over the same UART (like on the Pi) this would work.

Would it be possible to make the UARTs TX/RX lines useable for different 'purposes' like in my example above? Ofcourse, baudrates and settings need to be the same, but this is not a problem for me. Not sure if the hardware supports this or how much effort it would be, all I know is, this works both on APM based FCs and also on the Raspberry Pi linux computer.

digitalentity commented 7 years ago

MSP is bi-directional protocol by design. It sends out confirmations to each command.

Is there any problem in connecting SPRacingF3 to RPi via USB? Or using softserial for telemetry output?

Linjieqiang commented 7 years ago

If INAV can use Mavlink as communicating protocol not use MSP it will be fine.But it was a big change.

rodizio1 commented 7 years ago

MSP is bi-directional protocol by design. It sends out confirmations to each command.

True. But it works just fine like I described above and I didn't see any acknowledgements coming out of Cleanflight when sending MSP RC packets to it,

Is there any problem in connecting SPRacingF3 to RPi via USB? Or using softserial for telemetry output?

Softserial for telemetry output would be possible, but I only have one serialport on the Pi, so I'd be forced to the same low baudrate for R/C and telemetry.

The UART1/USB port on the Flightcontrol is already connected to a USB port of the Pi (for bi-directional MSP that will be forwarded to EZ-GUI).

digitalentity commented 7 years ago

The UART1/USB port on the Flightcontrol is already connected to a USB port of the Pi (for bi-directional MSP that will be forwarded to EZ-GUI).

Use that to inject MSP_RC packets into the flight controller and use UART only for telemetry data.

digitalentity commented 7 years ago

I don't see a point in implementing something that can be easily solved by better utilization of existing features.

rodizio1 commented 7 years ago

Use that to inject MSP_RC packets into the flight controller and use UART only for telemetry data.

This is not easily possible, as there is already a datastream being forwarded from the Pi on the aircraft to the Pi on the ground and from there to the EZ-GUI (and vice versa). Doing it like that would require me to add some complicated "serial-proxy" type of application, i.e forwarding the EZ-GUI data and also additionally injects the MSP RC data. Not very nice. I'd like to keep this seperated, so people can decide if they want to use EZ-Wifibroadcast R/C or "traditional" R/C and still use the EZ-GUI features.

I don't see a point in implementing something that can be easily solved by better utilization of existing features.

I realize that my EZ-Wifibroadcast Raspberry Pi thing is a somewhat unusual kind of application, however, it's being used by more and more people, so I think there could be some demand in the future.

And in general, my feeling is, having more hardware UARTs useable/available is always a good thing, after all, many people buy F3 based FCs just because of the additional UARTs, not because of CPU/FPU power.

digitalentity commented 7 years ago

I realize that my EZ-Wifibroadcast Raspberry Pi thing is a somewhat unusual kind of application, however, it's being used by more and more people, so I think there could be some demand in the future.

A few thoughs on this:

rodizio1 commented 7 years ago

Using one MSP stream to pass R/C from your app and the other MSP to task to EzGUI can lead to unsafe behavior if both your app and EzGUI will start sending MSP_RC frames. You have to implement filtering which is one step from becoming a true MSP proxy

Indeed, but I don't plan to use the R/C function of EZ-GUI anyway, it's experimental, and there are a lot of warning messages, so it's unlikely somebody will use it accidently.

MSP is bi-directional by design. Hacking it to become uni-directional is generally a bad idea which may lead to issues.

I've used that setup for a few flights, it works just fine.

You have enough serial ports to have two MSP streams plus telemetry plus GPS if you so desire. You just need to use them wisely

Again, this is not possible with my setup. Using softserial would limit my baudrate to 19.200 since I only have one UART on the Pi. Or I'd have to use an additional USB2Serial converter on the Pi, additional devices and wiring, also not very nice.

Like you say, the only viable solution would be some kind of "MSP proxy", but that just adds additional software-logic that complicates things and that can have bugs and fail. The solution I have now (generating MSP packets from Joystick-Input on the ground Pi, sending them to the Pi on the aircraft and sending them out on the serialport) is simple and proven.

digitalentity commented 7 years ago

The other viable solution is to talk MSP_RC via USB and use 19200 software serial for communication with EzGUI (works fine btw)

rodizio1 commented 7 years ago

Nope. I only have one serialport on the Pi. If I use that one for the bi-directional EZ-GUI link and an USB port for the MSP RC stuff, I don't have anything left for the LTM telemetry on the Pi.

digitalentity commented 7 years ago

Actually, you don't need LTM or any other telemetry - you have MSP which can provide you with all data you need. Use polling to pull what you need from FC (like MWOSD does). Ultimately only one USB connection to FC would be required.

stronnag commented 7 years ago

I would prefer that we did not open up the possibility of an attacker hijacking my aircraft over the telemetry port.

rodizio1 commented 7 years ago

The wifibroadcast_osd I use doesn't support MSP telemetry, only mavlink, LTM and Frsky and no polling. I'd have to add MSP support with polling, which is far from trivial. Apart from that, polling is not really useable with wifibroadcast, as upstream bandwidth is limited due to the underlying wifi medium access.

Digitalentity, I realize you guys have lots of stuff to do (and there are more urgent things), but I really have thought about this for weeks and tried all kinds of stuff. I wouldn't be bothering you with this feature request if there was a more viable solution or even a "not-too bad workaround" on my side. But as I see it, there isn't.

digitalentity commented 7 years ago

It's not that I'm entirely against this idea. I don't see a point in hacking MSP implementation to allow uni-directional communication. This creates all sorts of issues - here are a few that just came to me:

  1. A separate configuration setting to distinguish when to replace MSP with telemetry entirely and when to release only TX and keep listening on RX for MSP packets.

  2. MSP expects to be able to send response data. Hacking well-debugged implementation to ignore this is not a good idea considering very low demand for this feature.

  3. Considering upcoming bi-directional telemetry (MAVLink for starters) the feature you are requesting is becoming even less clean in terms of implementation.

I would really like to also know @martinbudden, @DzikuVx and @oleost 's oppinion on this feature.

digitalentity commented 7 years ago

@rodizio1 please take no offense, but I think it would be better if EZ-Wifibroadcast would adapt to support more different flight controllers. If you add canonical polled MSP OSD you'll instantly become compatible with all (with some caveats) MSP-talking firmwares starting from ancient MultiWii.

Similarly - when we wanted support for Crossfire telemetry - it was created in INAV instead of trying to convince TBS to better support something we already have.

oleost commented 7 years ago

I have not got a chance to read entire issue.

But as far I understand implementing mavlink will sort his issue?

And mavlink is already being wanted by multiple users and it would also open up a lot of ground stations for INAV.

So I vote for going forward with mavlink support.

rodizio1 commented 7 years ago

It's not that I'm entirely against this idea. I don't see a point in hacking MSP implementation to allow uni-directional communication. This creates all sorts of issues - here are a few that just came to me:

Sorry, but there is no "hack", it's already working fine with a "stock" Cleanflight version.

I simply took Cleanflight 1.14, configured it for MSP_RC on UARTx, connected that UARTs Rx pin to the Tx pin of the UART on the Pi and let the Pi send MSP RC packets over it. That's it. Works just fine, delay is low, high updates rates are possible, failsafe also works as expected.

please take no offense, but I think it would be better if EZ-Wifibroadcast would adapt to support more different flight controllers. If you add canonical polled MSP OSD you'll instantly become compatible with all (with some caveats) MSP-talking firmwares starting from ancient MultiWii

It already supports Mavlink, LTM and Frsky, I think that's already plenty of protocols. I would even spend the time to implement MSP one-way, but doing it polled is pointless with wifibroadcast, as every single request would have to be sent upstream over the air. The telemetry data is being parsed and rendered on the ground Pi. For the typical analog OSDs which sit on the aircraft, this is fine of course.

Of course, I could also write something that does the polling on the air Pi and only forwards the outgoing telemetry to the ground Pi (plus sending MSP RC packets from Pi to FC ...), but that would be a big-ass monster proxy ;) and just complicate everything and make it a lot more error-prone (especially with my bad C-skills ...)

The solution I proposed above would allow me to:

In a very simple and non-error-prone way and without having to resort to ugly solutions like an additional USB2Serial adapter on the aircraft.

rodizio1 commented 7 years ago

But as far I understand implementing mavlink will sort his issue?

And mavlink is already being wanted by multiple users and it would also open up a lot of ground stations for INAV.

No, this is not about telemetry protocols. I simply would like to not "waste" serial port hardware.

As it is now, when one UART is being used only uni-directional (for example the TX pin for outgoing LTM telemetry), the complete UART is "used-up" as the corresponding RX pin of that UART is not useable anymore for anything else.

rodizio1 commented 7 years ago

So far I see two arguments from your side, digitalentity:

I have explained why those options are not good options for my case. Apart from that, there might be other use cases that could benefit from this. Like I wrote, APM supports this, I figure they've added it because somebody had a use for it.

Again, MSP_SET_RAW_RC is not bi-directional, Cleanflight works just fine receiving MSP packets and does not send anything back.

It's not that I'm entirely against this idea. I don't see a point in hacking MSP implementation to allow uni-directional communication. This creates all sorts of issues - here are a few that just came to me:

To me it looks like you already made up your mind about this when you closed this issue (just 20mins after creation and without even bothering to wait for an answer ...) and then you look for technical arguments that support your gut decision. You, obviously working with computers, I'd expected to be more on the side of science ;)

image

@rodizio1 please take no offense, but I think it would be better if EZ-Wifibroadcast would adapt to support more different flight controllers.

Giving different ideas or approaches to find a solution is fine. Closing the issue after 20mins without knowing any details or even waiting for an answer I'd consider offensive though.

digitalentity commented 7 years ago

@rodizio1 I gave you several options to make your solution work without modifying the INAV code.

How many users do you have using your solution? I understand that it's easier for your to have somebody adapt INAV to your needs, but I will consider spending time modifying INAV to make it compatible with something that is widely adopted already, not with something that only a handfull of people use.

If you still feel that you need to modify INAV to suite your needs - please consider implementing it yourself and submitting a pull-request into INAV - this is how opensource development usually works.

Regarding other things. As I said before - your problem can be solved on your end without any hacks on INAV side, that was the very reason for closing this issue. No other technical evidence is necessary to support that decision.

Also, @oleost has a point. MAVLink (when it goes bidirectional) can also be used for telemetry and control and it will also occupy only one UART. It will happen naturally in 1.7 maybe you should consider migrating to MAVLink instead?

NiqueIt commented 7 years ago

Hi guys I see the point of you DE, but I prefer also to have this request solved and not closed. The main request is not to solve one specific solution. That wifi-broadcast-thing is one example.

The main request is to split TX from RX for advanced user they know what they are doing. There are some more cases where serial communication do not request a two-way communication. And with splitting, you have more space for unidirectional serial comm. Thats the request. If it is valid or not (MCP_RX) is not the question, it is the problem of the "advanced user".

What I see (and do not now if already possible), is to feed for example the telemetry to more than one recipient. That is my case. Another (ok, that is again very special in my case) is a multi pilot cockpit. Where you have two control streams to the air. I solved this with a so called "decision maker" in front of the FCU. The code is not a lot and if more comm-flexibility is there, makes room for unconventional advanced solutions. The decision maker is just the beginning of redundancy where finally 3 FCU should interact but only one is flying. There are so many cases where you need more communication. And don't say I have to go to other autopilots. I will for this. But not for all 3 units. Ideally, all 3 should be different. And why not one with INAV? May be really far from what is going here. But I shows, that there may be other situations than "just" rodizio's.

I recommend to thing again on the request: Splitting TX from RX, whatever users do then.

digitalentity commented 7 years ago

I don't mind sharing UARTs between something that is uni-directional by design (i.e. SerialRX and some telemetries). However MSP is bi-directional by design - why break the design?

Wait until MAVLink will be available in it's bi-directional form - we'll also allow RC control via MAVLink (part of the protocol).

DzikuVx commented 7 years ago

I just read whole conversation: MSP is bi-directional and by design is sending confirmation for each packet. That makes MSP requires whole UART with both TX and RX. Period.

I'm strongly against making special version of MSP protocol that does not sends response. Or rather, I'm against publishing it in any official release.

Bottom line: I agree with @digitalentity that this proposal should not be implemented in official release of INAV.

rodizio1 commented 7 years ago

I don't mind sharing UARTs between something that is uni-directional by design (i.e. SerialRX and some telemetries).

Great. This is exactly what I'm trying to do. Using a completely uni-directional serial rx protocol (well, it's MSP_RX in the code, but technically, it's a serial receiver) and a completely uni-directional telemetry protocol (LTM in my case) on the same serialport.

Nice, that we finally agree :)

However MSP is bi-directional by design - why break the design?

I just read whole conversation: MSP is bi-directional and by design is sending confirmation for each packet. That makes MSP requires whole UART with both TX and RX. Period.

Let me repeat once again. MSP_RX is not bi-directional. You guys do not need to hack anything. I am trying to use MSP_RX exactly as it was designed and intended, with the only difference being that I want to use the "wasted" TX line of that serialport for other things.

Again, I did not see any kind of ACK or whatever answer coming out of the serialport I've been using for MSP_RX and also not on the "main" serialport the Chrome-GUI usually connects to.

If you don't believe this, try it yourself with a serial-monitor, look into the code or simply read the Multiwii release notes from years ago:

http://www.multiwii.com/forum/viewtopic.php?f=8&t=6231

  • remove MSP ack for MSP_SET_RAW_RC, and only for this message

As you can see, there is no acknowledgement. Guess why they removed it. Because it doesn't make sense for that type of packet, what's the point in acking (or nacking) R/C control packets? It just uses up serial bandwidth and other resources and gives no extra value. Should the sender send it again if it didn't receive an ack from the flightcontrol in a timely manner? Well, the stick-positions are sent very often anyway, there's no point in sending "outdated" stick positions again. Or should the FC nack the packets? What would be the use-case for that?

Wait until MAVLink will be available in it's bi-directional form - we'll also allow RC control via MAVLink (part of the protocol).

This won't solve anything, would basically be the same as digitalentitys proposal of writing a msp-proxy. Yes, there is already mavproxy, but it's written in Python and uses way too much CPU. Then there is cmavnode, mavproxy written in C/C++, but doesn't have all the functionality, would need a lot of work.

And the general thing with mavlink R/C control is, that it seems to be more made for "occasionally flying your fully-featured automatic platform" manually, i.e. it has low update rates and is not so well suited for fast or more dynamic flying.

That's where MSP_RX shines, it behaves just like a serial RX like SBUS for example, fast update rates, low latency, and works simply by connecting a serial-line, no need to write complicated (and error-prone) mavlink or msp proxies.

Apart from that, there are other use-cases where "sharing" serial ports can be useful. GPS for example is one-way (if it is already configured correctly) blackbox also, these could be shared over one serialport. The same applies for other serial-rx protocols like Graupner or whatever.

rodizio1 commented 7 years ago

[crickets]

Here is a Cleanflight pull request, two different people implemented similar things, one for blackbox and serial_rx on same UART, the other for Frsky telemetry and serial_rx on the same UART.

https://github.com/cleanflight/cleanflight/pull/1475

stronnag commented 7 years ago

Irrelevent, these are uni-directional protocols.

rodizio1 commented 7 years ago

RX_MSP is uni-directional as well.

stronnag commented 7 years ago

RX_MSP does not exist as an entity, it's MSP.

rodizio1 commented 7 years ago

Yeah, I've been looking through the code. It looks like the MSP code for GUI etc. is being re-used also for RX_MSP.

But that still doesn't make RX_MSP bi-directional.

It looks to me that this was implemented like this because the MSP code was there anyway, and this was the easiest way. Not the "cleanest" ...

Edit: So actually, it's just the code that treats MSP_RX as bi-directional, although there is no good reason for that (apart from saving development time).

DzikuVx commented 7 years ago

@rodizio1 there is no RX_MSP protocol. RX_MSP is only a subset of MSP protocol. One MSP frame to be precise MSP_SET_RAW_RC , ID 200.

MSP_SET_RAW_RC can not exists without full MSP implementation and MSP (required for what you call RX_MSP) is bi-directional. Ergo, what you call RX_MSP is also bi-directional.

digitalentity commented 7 years ago

There is no (and there never was any) RX_MSP as a stand-alone entity. It was always part of MSP protocol which is bi-directional and sends confirmation for each received packet by design. MSP_SET_RAW_RC (aka MSP_RX) is not an exception.

MultiWii created a hacked version and removed the ack for MSP_SET_RAW_RC "to lower downlink BW" when the machine is controlled over wireless telemetry. They never had any intention to make it shareable with something else.

@rodizio1, we are not going to allow uni-directional MSP. However there are many ways to achieve what you so desire, let me name a few:

  1. You can adapt your software to better utilize possibilities of MSP for control and poll telemetry
  2. You can wait for bi-directional MAVLink and use it to transfer control and receive telemetry via the same UART
  3. You can create a separate SerialRX protocol, i.e. "EZWB" (EZ-WifiBroadcast) to receive MSP_RX and send out LTM telemetry on the same UART. Note, that this won't be part of MSP stack - for SerialRX you are free to do whatever you want. This solution is much more likely to be adopted in mainstream INAV than what you suggested.
  4. If all of the above is not an option for you - you are free to fork INAV and hack it any way you like. It's how open source software works.
martinbudden commented 7 years ago

Actually, I think option (3) is quite a good one.

digitalentity commented 7 years ago

Yes. It's still not a pretty solution, but at least it doesn't break established protocols.

rodizio1 commented 7 years ago

I realize MSP_RX is not a protocol, I mean the functionality. If Cleanflight/Inav really sends acks for MSP_SET_RAW_RC (I haven't seen any ... but maybe I did something wrong as I wasn't really expecting them as I assumed the multiwii code also made it into cleanflight) that still makes no sense. It's completely pointless to send acks for R/C packets.

The clean solution would be number 3 IMHO, thats similar to what I have been asking for. Just no need to call it EZWBC protocol, as it is nothing specific to EZWBC, there are a lot of other people and DIY projects who use MSP_RX exactly like I do with wifibroadcast, i.e. "just send MSP_SET_RAW_RC packets, don't care for acks". I'd just throw the old MSP_RX stuff out and replace it with a serial RX that is written exactly like the other serial RXes like SBUS.

Well, anyway, you don't want to touch MSP, so I looked at some other options in the meantime that are independent from MSP.

I don't mind sharing UARTs between something that is uni-directional by design (i.e. SerialRX and some telemetries).

So you would consider changing inav so that it allows sharing between the different serial rx protocols and uni-directional telemetry like LTM?

That would be a good option IMHO. I'd have to completely rewrite my stuff to use SUMD instead, but that's doable and won't compromise stability and reliability on my side like a MSP-proxy solution.

On your side no need to touch MSP/MSP_RX, no need to introduce another serialrx. Just the sharing needs to be done.

That would make me with my wifibroadcast setup happy and also other people like in the cleanflight pull request:

This feature is quite useful in simple OpenLRSng based receivers. Original FrSky runs at 9600 baud, in this scenario it runs at the same speed as SerialRX. But it's not a problem for OpenLRSng, because once using serial rx mode, it still accepts telemetry data at serial rx speed. So OpenLRSng can read telemetry data at eg 115200 and transfer it.

I've tested it with naze32 - uart2 for SUMD and FrSky telemetry set, then it goes to my μLRS receiver. I can see the telemetry data back on my Taranis.

digitalentity commented 7 years ago

So you would consider changing inav so that it allows sharing between the different serial rx protocols and uni-directional telemetry like LTM?

Please re-read what I proposed for option 3. It's not sharing an UART between SerialRX and telemetry. I suggest having a SerialRX protocol which sends out it's own telemetry on it's dedicated UART. TBS Crossfire protocol (CRSF) does precisely that.

digitalentity commented 7 years ago

Sharing a SerialRX with uni-directional telemetries might be doable, although I still believe that a proper solution should involve hardware that is able to use protocols as they were designed to be.

digitalentity commented 7 years ago

I've renamed the issue and re-opened it so we won't lose the whole conversation.

rodizio1 commented 7 years ago

Please re-read what I proposed for option 3. It's not sharing an UART between SerialRX and telemetry. I suggest having a SerialRX protocol which sends out it's own telemetry on it's dedicated UART. TBS Crossfire protocol (CRSF) does precisely that.

This is basically the same:

  1. You can create a separate SerialRX protocol, i.e. "EZWB" (EZ-WifiBroadcast) to receive MSP_RX and send out LTM telemetry on the same UART. Note, that this won't be part of MSP stack - for SerialRX you are free to do whatever you want. This solution is much more likely to be adopted in mainstream INAV than what you suggested.

I.e. "invent" a protocol that receives e.g. SUMD R/C in one direction and sends out e.g. LTM telemetry in the other direction and call it "EZ-whatever".

Only difference to my proposal would be that your proposal would only be beneficial for EZ-Wifibroadcast users and that specific use-case, while my proposal of a generic serial-port-sharing-feature would be beneficial for a lot more users and use-cases.

Regarding TBS and their CRSF: For TBS, establishing a new R/C and telemetry protocol made sense. PPM is analog and slow and all the other protocols are made by competitors (SBUS, SUMD, etc.) or are not widely supported (MSP). For telemetry it's the same, Mavlink uses too much bandwidth, LTM is not widely supported and Frsky is a competitor. Apart from that, it always makes sense for a company to establish "their own" protocol, it's free advertisement in all other products and open source projects that support their protocol. And there is a chance that it becomes the "de-facto standard".

flyingoose commented 7 years ago

Apologies for the noise, but... +1 to be able to use one UART for SBus and SmartPort, basically connect FrSky XSR to one UART.

digitalentity commented 7 years ago

@flyingoose not possible. SBus has baud rate of 100000 and is unidirectional. S.Port uses 57600 baud rate and is bi-directional. They can't share one UART.

krzysztofmatula commented 7 years ago

As SBus is 100kbps, sharing UART with any uni-directional telemetry would requite that telemetry to run on such non-standard rate... Not good...

But..., what about remapping the TX pin from UART to regular GPIO and setting a softserial there? Is it possible with the current chips?