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.

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

fest(flysky): Flysky Gimbal 2.0 Sync Sampling Support #5869

Open
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

richardclli
Copy link
Collaborator

@richardclli richardclli commented Feb 5, 2025

Flysky works out a new version of firmware that can support sync sampling:

  1. Upon init the gimbal will operate in backward compatible mode (i.e. old EdgeTX will works after the gimbal firmware is upgraded)
  2. New firmware will check the version of the firmware to see if sync mode is supported
  3. New driver will send command to the gimbal for sync sampling

The gimbal driver changes:

  1. It will automatic obtain the version of the gimbal to check if sync sampling is available
  2. The default operation mode is backward compatible mode, will switch to sync modes only when new version of gimbal firmware is detected
  3. Based on sampling frequency, the driver will automatically switch to sync sampling mode (<= 400Hz - switching threshold) or sync resampling mode (> 400Hz)

Testing:

  1. Not breaking the operation of existing old gimbals including the gimbal of GX12
  2. Flysky firmware seems to have some problems now, trying to verifying with them.

@richardclli richardclli added this to the 2.11 milestone Feb 5, 2025
@richardclli richardclli self-assigned this Feb 5, 2025
@richardclli richardclli marked this pull request as draft February 5, 2025 08:55
@richardclli richardclli force-pushed the feat-flysky-gimbal-sync-sampling branch 2 times, most recently from 3d0a61f to be0b0ee Compare February 7, 2025 02:29
@richardclli richardclli marked this pull request as ready for review February 7, 2025 07:04
@richardclli richardclli force-pushed the feat-flysky-gimbal-sync-sampling branch from d42f76f to 886d3f0 Compare February 7, 2025 07:12
@richardclli richardclli force-pushed the feat-flysky-gimbal-sync-sampling branch from aa547f3 to 71408e9 Compare February 8, 2025 01:39
Copy link
Member

@raphaelcoeffic raphaelcoeffic left a comment

Choose a reason for hiding this comment

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

I haven't checked the protocol itself, only the sync trigger / wait for now.

@@ -58,8 +66,21 @@ static const etx_serial_port_t _fs_gimbal_serial_port = {

static STRUCT_HALL HallProtocol = { 0 };

static uint8_t _fs_hall_command[8] __DMA;
Copy link
Member

Choose a reason for hiding this comment

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

DEFINE_STM32_SERIAL_PORT() already provides for TX DMA buffers in the last parameter. Please use that instead. This will declare the buffer with the proper memory type (here __DMA_NO_CACHE), which is important for H7 targets.

Comment on lines +257 to +264
// Need to put all in a group to ensure DMA transfer will not affect ADC sampling
#if defined(FLYSKY_GIMBAL) && !defined(SIMU)
flysky_gimbal_start_read();
#endif
#if defined(FLYSKY_GIMBAL) && !defined(SIMU)
flysky_gimbal_wait_completion();
#endif

Copy link
Member

Choose a reason for hiding this comment

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

Ideally, this should be encapsulated in a way that does not require conditional compilation (no #if defined(...)). One way would be craft a special ADC driver, or providing new hooks, either in the mixer and/or in generic ADC code.

Comment on lines +311 to +320
if(_fs_gimbal_detected && _fs_gimbal_version > GIMBAL_V1 && _fs_gimbal_mode != V1_MODE) {
auto timeout = timersGetUsTick();
while(!_fs_gimbal_cmd_finished) {
// busy wait
if ((uint32_t)(timersGetUsTick() - timeout) >= SAMPLING_TIMEOUT_US) {
// TRACE("Gimbal timeout");
return;
}
}
}
Copy link
Member

Choose a reason for hiding this comment

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

We should really prevent busy waits in mixer task. This will add additional time during which the mixer is hogging the MCU and preventing other tasks to run.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants