edgetx icon indicating copy to clipboard operation
edgetx copied to clipboard

chore: refactor "Flysky gimbal" to "Serial gimbal"

Open rotorman opened this issue 11 months ago • 14 comments

rotorman avatar Dec 16 '24 09:12 rotorman

The issue I can see is there might be other serial gimbal protocols looming around..

3djc avatar Dec 16 '24 09:12 3djc

As the same proto was used by also other companies as FS, the name seemed not appropriate anymore. Would you have a better suggestion how to work around this?

rotorman avatar Dec 16 '24 09:12 rotorman

As the same proto was used by also other companies as FS, the name seemed not appropriate anymore.

IMO... yes and no... if the same protocol is being used... and Flysky came up with it first... then it is still Flysky serial protocol... but I think perhaps it is more that new radios, i.e. GX12 is using serial gimbals, not necessarily Flysky protocol, so instead a new specifier is needed to cover that case, or at least a generic specifier. i.e. perhaps "flysky serial gimbal" is a subtype of "serial gimbal" ;)

pfeerick avatar Dec 16 '24 10:12 pfeerick

As the same proto was used by also other companies as FS, the name seemed not appropriate anymore.

IMO... yes and no... if the same protocol is being used... and Flysky came up with it first... then it is still Flysky serial protocol... but I think perhaps it is more that new radios, i.e. GX12 is using serial gimbals, not necessarily Flysky protocol, so instead a new specifier is needed to cover that case, or at least a generic specifier. i.e. perhaps "flysky serial gimbal" is a subtype of "serial gimbal" ;)

GX12 is using flysky serial gimbal protocol, I was thinking about another manufacturer which serial gimbal will use a completely different protocol. I'm personally not too fussed that the first to bring up a feature/standard/code gets his name for it, we call CRSF the radio to module ELRS protocol too

3djc avatar Dec 17 '24 14:12 3djc

I have concern about this. Flysky has a new version of protocol which supports sync sampling. And I am working on it, and I cannot guarantee that this change is compatible with other serial gimbals. I think the best way is to split the implementations.

richardclli avatar Dec 18 '24 08:12 richardclli

That's a possibility, but then how do we select which one to use ? Ok, Flysky radio get FS one, RM rm; Jumper jumper, but what does it mean for say FrSky radio that wants to add one of those 3. Do we include none and provide a compilation option for user to build their own custom firmware ?

3djc avatar Dec 18 '24 09:12 3djc

Also, what if non-FS radio is modded with FS gimbals, like is possible: https://github.com/EdgeTX/edgetx/wiki/Flysky-Hall-Sticks-Mod

Ideally, our code should auto-detect the gimbal attached.

rotorman avatar Dec 18 '24 09:12 rotorman

Ideally I would agree, but I don't think any of those protocols actually allows that

3djc avatar Dec 18 '24 09:12 3djc

The present FS implementation has a checksum, which could be one way to validate if present non-sync FS protocol is being used, or not.

rotorman avatar Dec 18 '24 09:12 rotorman

RM serial gimbal use 100% the same code, but for synchronisation where they use a dedicated line, so at this point in time, I have forced it on GX12 to assume it is a RM gimbal, so it uses Flysky serial code, and forces RM synchronization

3djc avatar Dec 18 '24 09:12 3djc

And my opinion is not necessary to do this renaming at this moment. RM just made a Flysky protocol compatible gimbal with sync enhancement. Not quite original to worth any refactoring effort just for a name!

richardclli avatar Dec 21 '24 00:12 richardclli

@rotorman @richardclli @3djc

Can we get consensus on this?

I am of the opinion of "flysky gimbals" can be a subset of "serial" gimbals, but this is contingent on a second serial gimbal being present, requiring slightly different handling. Otherwise, very much, whatever.

pfeerick avatar Apr 28 '25 04:04 pfeerick

I am with you that the top-level description for serially attached gimbals should be SerialGimbals, which could then have subdescriptors for variants, like SerialGimbalFS/SerialGimbalFlysky and so on.

rotorman avatar Apr 28 '25 10:04 rotorman

@rotorman @richardclli @3djc

Can we get consensus on this?

I am of the opinion of "flysky gimbals" can be a subset of "serial" gimbals, but this is contingent on a second serial gimbal being present, requiring slightly different handling. Otherwise, very much, whatever.

First take a look at https://github.com/EdgeTX/edgetx/pull/5869

Now there are gimbals that implement:

  1. subset of Flysky v1.0 protocol (GX12)
  2. Flysky v1.0 protocol (Flysky radios)
  3. Flysky v2.0 protocol (Flysky gimbal update)

Now auto detection is only done for Flysky v2.0 protocol, maybe we need to figure out a way to distinguish 1 and 2 as well. And I think the change should be made on top of #5869

richardclli avatar May 06 '25 01:05 richardclli

Why the Flysky use a mcu to conver mlx90393 signal from spi to serial, why not use the on board mcu to read the mlx90393 via spi?

kkbin505 avatar Jun 26 '25 00:06 kkbin505

Why the Flysky use a mcu to conver mlx90393 signal from spi to serial

If I had to guess, it is probably related to the gimbal calibration process, as the gimbals are individually calibrated, not tied to the handset. Similar thing applies to the RM GX12, although that is a bit of a hybrid.

pfeerick avatar Jun 26 '25 03:06 pfeerick