RC_TYPE for binding #2163
RC_TYPE for binding #2163
Conversation
|
Thanks @julianoes But would the approach in #2162 be better? That essentially makes the param 1 into RC_TYPE and then suggests sub-types. I'd really like to know if RC_TYPE is ever useful - from a vehicle point of view you just want to know that you should start the binding process - how that is done would depend on the vehicles knowledge of what RC it had attached. |
|
RC sub types 🤯, I'd try to keep it simple. |
|
@julianoes Hmmm, my immediate response was agreement, but then I checked that PX4 existing behaviour parses this as a type and sub-type https://github.com/PX4/PX4-Autopilot/blob/main/src/drivers/px4io/px4io.cpp#L545-L578 If you look at that code it also shows that the flight stack does do different things for the different RC types. Now I think that is all the wrong way round - we should just sent "bind": the flight stack should know what RC it has and configure itself appropriately. But that's not what we have and we can't break that implementation. Further, if we add this rc type, then we also need to specify what the param 1 should be in this case - it can't be zero because PX4 would parse that as a spektrum. Upshot, I think that we have to effectively define a type and subtype, to not break anything, which means that we need to look at the other PR. Hope that makes sense. I'm closing this, because sadly the simple way doesn't work without making it into the hard way (i.e. I could make this param 1 != 0 mean "undefined", but that's even more complicated. |
|
Obviously a little thinking required.But specifically for this: “CRSF” is a wire protocol, not an RC type. We’d need to confirm that RC “types” that implement CRSF all behave the same for binding, or document how this is actually going to work.On 7 Nov 2024, at 09:04, Hamish Willee ***@***.***> wrote:
@julianoes Hmmm, my immediate response was agreement, but then I checked that PX4 existing behaviour parses this as a type and sub-type https://github.com/PX4/PX4-Autopilot/blob/main/src/drivers/px4io/px4io.cpp#L545-L578
If you look at that code it also shows that the flight stack does do different things for the different RC types. Now I think that is all the wrong way round - we should just sent "bind": the flight stack should know what RC it has and configure itself appropriately. But that's not what we have and we can't break that implementation.
Further, if we add this rc type, then we also need to specify what the param 1 should be in this case - it can't be zero because PX4 would parse that as a spektrum.
Upshot, I think that we have to effectively define a type and subtype, to not break anything, which means that we need to look at the other PR.
Hope that makes sense. I'm closing this, because sadly the simple way doesn't work without making it into the hard way (i.e. I could make this param 1 != 0 mean "undefined", but that's even more complicated.
Would it change your mind if I pointed out that this is how PX4 handles the types https://github.com/PX4/PX4-Autopilot/blob/main/src/drivers/px4io/px4io.cpp#L545-L578 ?
I mean it doesn't have to, but i
@auturgy @benjinne In that case my take is that there is no specific handling of this message for a particular RC type currently, because the
can I reasonably make it even more simple like this?
<entry value="500" name="MAV_CMD_START_RX_PAIR" hasLocation="false" isDestination="false">
<description>Starts receiver pairing.</description>
<param index="1" label="All">0: Bind.</param>
<param index="2" label="RC Type" enum="RC_TYPE">Bind specific (optional)</param>
</entry>
<enum name="RC_TYPE">
<description>RC type. Used in MAV_CMD_START_RX_PAIR.</description>
<entry value="0" name="RC_TYPE_SPEKTRUM_DSM2">
<description>Spektrum DSM2</description>
</entry>
<entry value="1" name="RC_TYPE_SPEKTRUM_DSMX">
<description>Spektrum DSMX</description>
</entry>
<entry value="2" name="RC_TYPE_CRSF">
<description>CRSF radio</description>
</entry>
</enum>
—Reply to this email directly, view it on GitHub, or unsubscribe.You are receiving this because you were mentioned.Message ID: ***@***.***>
|
CRSF is indeed a wire protocol, As defined in the protocol, binding is a feature supported. It depends on the receiver that implements it if it's supported. Currently I'm only aware of ELRS 3.4+ that works. I assume crossfire supports it earlier |
This adds a type for binding CRSF RC radios, which is being supported in PX4/PX4-Autopilot#23294
There is an alternative proposal #2162 this is probably better - but I created this before I saw that. Lets discuss in mav call.
@benjinne I've described this as "CRSF radio". The way I saw this message is that is was poorly designed originally with just spectrum and then the other options were added later with RC_TYPE when someone wanted to implement those. In other words it wasn't a design to go type/and subtype. I could be wrong though.
FWIW I'm not even sure having an RC type is actually useful. You could just say "I get this command, then I check what types of RC radios I have, and if they support binding, I attempt to bind them. Too late to change now I guess - we just need to work out how to take this forward.
Note, the RC type isn't even useful for the two RC on vehicle case, since they might be the same kind.
@auturgy @julianoes
FMI Is it ever useful to specify the RC "kind" here?