Fix SITL target build and runtime segfaults #9342

metametaclass · 2020-01-06T14:42:55Z

SITL target segfaults in several places after running because of absent motorPwmDevice functions.
Target cannot be built with DEBUG=GDB option because of absent uartPinConfigure and IOConfigGPIO function calls.
Entering CLI tab in connected to SITL configurator also causes segfault because of some OSD cli settings placed outside of #ifdef USE_OSD

ledvinap · 2020-01-06T19:08:35Z

src/main/cli/settings.c


    { "osd_rcchannels",             VAR_INT8   | MASTER_VALUE | MODE_ARRAY, .config.array.length = OSD_RCCHANNELS_COUNT, PG_OSD_CONFIG, offsetof(osdConfig_t, rcChannels) },
    { "osd_camera_frame_width",     VAR_UINT8  | MASTER_VALUE, .config.minmaxUnsigned = { OSD_CAMERA_FRAME_MIN_WIDTH, OSD_CAMERA_FRAME_MAX_WIDTH }, PG_OSD_CONFIG, offsetof(osdConfig_t, camera_frame_width) },
    { "osd_camera_frame_height",    VAR_UINT8  | MASTER_VALUE, .config.minmaxUnsigned = { OSD_CAMERA_FRAME_MIN_HEIGHT, OSD_CAMERA_FRAME_MAX_HEIGHT }, PG_OSD_CONFIG, offsetof(osdConfig_t, camera_frame_height) },
+#endif // end of #ifdef USE_OSD


Unrelated to this PR, but valid

Good find - but can this please be extracted into a separate pull request to keep track of it?

ledvinap · 2020-01-06T19:09:08Z

src/main/fc/init.c

@@ -538,7 +538,7 @@ void init(void)
    busSwitchInit();
 #endif

-#if defined(USE_UART)
+#if defined(USE_UART) && !defined(SIMULATOR_BUILD)


There is stub uartPinConfigure for SIMULATOR_BUILD

We should either remove this, or remove the stub.

s-ol · 2020-01-11T16:03:34Z

src/main/target/SITL/target.c

+{
+    UNUSED(io);
+    UNUSED(cfg);
+    printf("IOConfigGPIO\n");


alternatively, it's possible to add #undef's for USE_PINIO, USE_PINIOBOX, USE_PIN_PULL_UP_DOWN (see https://github.com/betaflight/betaflight/pull/9352/files#diff-f682e0332717b305936b703833517366)

I am surprised that this even built before these stubs were added - are we missing some compiler settings to make this fail for the default build?

@mikeller for me it didn't build without my change - I think it just hasn't been attempted to build in a while.

It is built as part of our CI on Travis and on Jenkins (see https://ci.betaflight.tech/job/Betaflight/2018/artifact/obj/betaflight_4.2.0_SITL-2018-6433aaa68.hex). Unfortunately we do not have strong checks in place for the compiler suite version used for this target, so I suspect the build problems are gcc version dependent.

I am surprised that this even built before these stubs were added - are we missing some compiler settings to make this fail for the default build?

It builds with make TARGET=SITL, but fails with make TARGET=SITL DEBUG=GDB. I think that in release build some of PINIO-related calls is optimized away.

s-ol · 2020-01-11T17:42:58Z

I've been passing through joystick input via MSP on a second TCP-UART, but I'm seeing some crazy latency in the receiver tab in the configurator. Is that the SITL build being slow, the TCP bridge being crappy, or the python MSP implementation I'm using being late?

I also haven't been able to get any PWM data out, I'm trying to listen using netcat: nc -luk 9002

mikeller · 2020-01-12T12:19:38Z

src/main/cli/settings.c


    { "osd_rcchannels",             VAR_INT8   | MASTER_VALUE | MODE_ARRAY, .config.array.length = OSD_RCCHANNELS_COUNT, PG_OSD_CONFIG, offsetof(osdConfig_t, rcChannels) },
    { "osd_camera_frame_width",     VAR_UINT8  | MASTER_VALUE, .config.minmaxUnsigned = { OSD_CAMERA_FRAME_MIN_WIDTH, OSD_CAMERA_FRAME_MAX_WIDTH }, PG_OSD_CONFIG, offsetof(osdConfig_t, camera_frame_width) },
    { "osd_camera_frame_height",    VAR_UINT8  | MASTER_VALUE, .config.minmaxUnsigned = { OSD_CAMERA_FRAME_MIN_HEIGHT, OSD_CAMERA_FRAME_MAX_HEIGHT }, PG_OSD_CONFIG, offsetof(osdConfig_t, camera_frame_height) },
+#endif // end of #ifdef USE_OSD


Good find - but can this please be extracted into a separate pull request to keep track of it?

mikeller · 2020-01-12T12:19:58Z

src/main/fc/init.c

@@ -538,7 +538,7 @@ void init(void)
    busSwitchInit();
 #endif

-#if defined(USE_UART)
+#if defined(USE_UART) && !defined(SIMULATOR_BUILD)


We should either remove this, or remove the stub.

mikeller · 2020-01-12T12:21:41Z

src/main/target/SITL/target.c

+{
+    UNUSED(io);
+    UNUSED(cfg);
+    printf("IOConfigGPIO\n");


I am surprised that this even built before these stubs were added - are we missing some compiler settings to make this fail for the default build?

mikeller · 2020-01-12T19:48:23Z

Can you please rebase / squash these changes into one commit?

…vice vTable functions and IOConfigGPIO stub

metametaclass · 2020-01-12T20:18:07Z

Can you please rebase / squash these changes into one commit?
Done.

Fix SITL target build and runtime segfaults

ledvinap previously approved these changes Jan 6, 2020

View reviewed changes

etracer65 mentioned this pull request Jan 11, 2020

Fix SITL build #9352

Closed

s-ol reviewed Jan 11, 2020

View reviewed changes

mikeller requested changes Jan 12, 2020

View reviewed changes

mikeller added the RN: BUGFIX label Jan 12, 2020

mikeller added this to the 4.1.3 milestone Jan 12, 2020

metametaclass dismissed ledvinap’s stale review via decac1f January 12, 2020 19:26

metametaclass requested a review from mikeller January 12, 2020 19:32

mikeller previously approved these changes Jan 12, 2020

View reviewed changes

mikeller added the Rebase / Squash required label Jan 12, 2020

fixed SITL target debug build and runtime segfaults. Added motorPwmDe…

548379b

…vice vTable functions and IOConfigGPIO stub

metametaclass dismissed mikeller’s stale review via 548379b January 12, 2020 20:17

metametaclass force-pushed the fix_SITL branch from decac1f to 548379b Compare January 12, 2020 20:17

mikeller approved these changes Jan 13, 2020

View reviewed changes

mikeller removed the Rebase / Squash required label Jan 13, 2020

mikeller merged commit b5237bb into betaflight:master Jan 13, 2020

mikeller added a commit that referenced this pull request Jan 13, 2020

Fix SITL target build and runtime segfaults (#9342)

9596d9a

Fix SITL target build and runtime segfaults

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix SITL target build and runtime segfaults #9342

Fix SITL target build and runtime segfaults #9342

metametaclass commented Jan 6, 2020

ledvinap Jan 6, 2020

mikeller Jan 12, 2020

ledvinap Jan 6, 2020

mikeller Jan 12, 2020

s-ol Jan 11, 2020 •

edited

mikeller Jan 12, 2020

s-ol Jan 12, 2020

mikeller Jan 12, 2020

metametaclass Jan 12, 2020

s-ol commented Jan 11, 2020 •

edited

mikeller Jan 12, 2020

mikeller Jan 12, 2020

mikeller Jan 12, 2020

mikeller commented Jan 12, 2020

metametaclass commented Jan 12, 2020

Fix SITL target build and runtime segfaults #9342

Fix SITL target build and runtime segfaults #9342

Conversation

metametaclass commented Jan 6, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

s-ol Jan 11, 2020 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

s-ol commented Jan 11, 2020 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mikeller commented Jan 12, 2020

metametaclass commented Jan 12, 2020

s-ol Jan 11, 2020 •

edited

s-ol commented Jan 11, 2020 •

edited