Call serial_free when Serial object is destroyed

[mmahadevan108]

Signed-off-by: Mahadevan Mahesh [email protected]

[mmahadevan108]

@geky

[geky]

Looks good to me 👍, we'll just have to let CI run.

previous pr: #1653
cc @0xc0170

[sg-]

@mbed-bot: TEST

HOST_OSES=windows
BUILD_TOOLCHAINS=GCC_ARM,ARM,IAR
TARGETS=K64F,NRF51_DK,NRF51_MICROBIT,NUCLEO_F411RE

[sg-]

/morph test

[mbed-bot]

[Build ${MBED_BUILD_ID}]
FAILURE: Something went wrong when building and testing.

[sg-]

@mbed-bot: TEST

HOST_OSES=windows
BUILD_TOOLCHAINS=GCC_ARM,ARM,IAR
TARGETS=K64F,NRF51_DK,NRF51_MICROBIT,NUCLEO_F411RE

[mbed-bot]

[Build 697]
SUCCESS: Building succeeded and tests were run! Be sure to check the test results

[mbed-bot]

Result: FAILURE

Your command has finished executing! Here's what you wrote!

/morph test

Output

mbed Build Number: 514

Test failed!

[bridadan]

The bot response is a bit misleading, sorry about that. The actual failure occurred in the build. The following targets failed to build for all three toolchains: MAX32600MBED and MAXWSNENV.

They each fail when linking tests with the following error:

ARM

16:02:22 Building project features-frameworks-utest-tests-unit_tests-test_setup_failure (MAXWSNENV, ARM)
16:02:22 Scan: ARM
16:02:22 Scan: test_setup_failure
16:02:23 Compile: main.cpp
16:02:23 Link: features-frameworks-utest-tests-unit_tests-test_setup_failure
16:02:23 Error: L6218E: Undefined symbol serial_free (referred from SerialBase.o).

GCC_ARM

16:03:55 Building project features-frameworks-utest-tests-unit_tests-test_setup_failure (MAXWSNENV, GCC_ARM)
16:03:55 Scan: GCC_ARM
16:03:55 Scan: test_setup_failure
16:03:56 Compile: main.cpp
16:03:56 Link: features-frameworks-utest-tests-unit_tests-test_setup_failure
16:03:56 C:/Jenkins/workspace/bm_wrap/156/.build/tests/MAXWSNENV/GCC_ARM/hal/common/SerialBase.o: In function `mbed::SerialBase::~SerialBase()':
16:03:56 SerialBase.cpp:(.text._ZN4mbed10SerialBaseD2Ev+0xa): undefined reference to `serial_free'
16:03:56 collect2.exe: error: ld returned 1 exit status

IAR

15:59:43 Building project features-frameworks-utest-tests-unit_tests-test_setup_failure (MAX32600MBED, IAR)
15:59:43 Scan: IAR
15:59:43 Scan: test_setup_failure
15:59:44 Compile: main.cpp
15:59:44 Link: features-frameworks-utest-tests-unit_tests-test_setup_failure
15:59:44 Error[Li005]: no definition for "serial_free" [referenced from C:\Jenkins\works
15:59:44           pace\bm_wrap\156\.build\tests\MAX32600MBED\IAR\hal\common\SerialBase.
15:59:44           o]

[geky]

@bridadan, that's very interesting. Is this a bug with the target implementations?

It looks like the MAXWSNENV/MAX32600MBED have simply not implemented the serial_free function.

[bridadan]

@geky Yup I looks like that's the issue!

[sg-]

just need to add some implementation there for the targets

[sg-]

/morph test

[mbed-bot]

Result: FAILURE

Your command has finished executing! Here's what you wrote!

/morph test

Output

mbed Build Number: 528

Test failed!

[sg-]

@0xc0170

[sg-]

@mbed-bot: TEST

HOST_OSES=windows
BUILD_TOOLCHAINS=GCC_ARM,ARM,IAR
TARGETS=K64F,NRF51_DK,NRF51_MICROBIT,NUCLEO_F411RE

[0xc0170]

Let's integrate this.

Future request, we might or not consider rule of 3 for SerialBase (I think this question was asked earlier in the previous PR), and document what is expected from serial_free as part of HAL.
Plus those empty functions for those targets, either implement right away or create an issue and notify owners that this shall be implemented.

[mbed-bot]

[Build 717]
SUCCESS: Building succeeded and tests were run! Be sure to check the test results

[sg-]

Looks like this caused a testing regression. Probably due to fflush not complete on dtor (guess) in the test cases. Need to hold on this one and investigate further.

[mmahadevan108]

Any update?

[0xc0170]

Any update?

This needs more than just a destructor. Serial handles resources, and we shall not destroy them just like that. We allow copy of Serial for instance, therefore creating a copy of Serial object, and destructing the old copy would lead to undefined behaviour (the new copy would use an object that might not be valid anymore - peripheral already disabled). We should provide a guideline for lifetime of objects.

We will keep you updated.

[mmahadevan108]

Thanks for your reply. The change does nothing new for implementations that have any empty serial_free() implementation.

It is only for the benefit of platforms that have some cleanup needed and has it implemented in serial_free() hal api.

[theotherjimmy]

Please reopen when unblocked.