Prevent ESP8266 stopping HW control on init

[michalpasztamobica]

Description

When trying to connect ESP8266 to WiFi, after it was previously disconnect, AT commands time out. This does not occur if CTS/RTS is disabled (for example, by not configuring the pins in mbed_app.json or hardcoding them to NC in ESP8266 class constructor). This also does not occur if we do not stop the HW control inside _init() when reconnecting. The stop function was anyway immediately overwritten by the call to start_uart_hw_flow_ctrl(), right after the reset, so the change should be otherwise neutral.

Pull request type

[x] Fix
[ ] Refactor
[ ] Target update
[ ] Functionality change
[ ] Docs update
[ ] Test update
[ ] Breaking change

Reviewers

@SeppoTakalo, @VeijoPesonen , please review.

[ciarmcom]

@michalpasztamobica, thank you for your changes.
@VeijoPesonen @SeppoTakalo@ARMmbed/mbed-os-ipcore @ARMmbed/mbed-os-maintainers please review.

[michalpasztamobica]

@VeijoPesonen , I tested the scenario you mentioned in a discussion: if I press the ESP reset button between the disconnection, the chip still reconnects.
I think the only scenario that would justify the usage of the stop_hw_uart_flow_control function would be that the MCU resets and runs without the UART HW control pins, while ESP is not reset and still runs with the CTS/RTS configuration. I wouldn't consider this a possible scenario, though...

[cmonr]

@michalpasztamobica question.

Will this change also be getting backported to https://github.com/ARMmbed/esp8266-driver, since the original code was introduced here #8689?

I think the only scenario that would justify the usage of the stop_hw_uart_flow_control function would be that the MCU resets and runs without the UART HW control pins, while ESP is not reset and still runs with the CTS/RTS configuration. I wouldn't consider this a possible scenario, though...

Is there a reason we don't want to consider the use case where the MCU isn't communicating with flow control, or am I misunderstanding this statement?

[SeppoTakalo]

@cmonr We don't backport to that repository as we only support one version of the ESP driver, and it is always the latest one in Mbed OS repository.

There is no reason to use the driver from that repository as it is just older version of this one. All development now happens in Mbed OS repository.

[michalpasztamobica]

@cmonr , answering your question:

Is there a reason we don't want to consider the use case where the MCU isn't communicating with flow control, or am I misunderstanding this statement?

Normally the reset of the MCU will also reset the ESP board - in this case both will run with or without the flow control, depending solely on the mbed_app.json config. We support both scenarios.

The unrealistic scenario, which I mentioned and which we would not handle, would be that the MCU resets with a modified flow control configuration while the ESP does not reset. If the MCU has a different flow control configuration than ESP8266, they would never be able to communicate.
I think that running into this situation is more of a design issue on the user's side and I would not worry about it. It could also lead to other problems (ESP unexpectedly feeding incoming data, as it is already connected to a network or reporting unexpected network status).
Does this makes things clear?

[JanneKiiskila]

@adbridge @0xc0170 - please target to next patch release.

[0xc0170]

CI started

[mbed-ci]

Test run: FAILED

Summary: 1 of 11 test jobs failed
Build number : 1
Build artifacts

Failed test jobs:

• jenkins-ci/mbed-os-ci_greentea-test

[cmonr]

Will need to restart once CI issue is resolved.

[cmonr]

#9173 (comment)
Does this makes things clear?

@michalpasztamobica It absolutely does, thank you for clarifying!

[JanneKiiskila]

@adbridge - one more push, can we get this merged in please?

[0xc0170]

@adbridge - one more push, can we get this merged in please?

It was waiting for release candidate to complete, now it can proceed.
Restarted CI (accidentally entire build), should be quick as this is second in the queue now

[0xc0170]

Exporter failure is being looked at , not related here

[0xc0170]

Exporter restarted

[mbed-ci]

Test run: FAILED

Summary: 3 of 11 test jobs failed
Build number : 2
Build artifacts

Failed test jobs:

• jenkins-ci/mbed-os-ci_dynamic-memory-usage
• jenkins-ci/mbed-os-ci_exporter
• jenkins-ci/mbed-os-ci_greentea-test

[cmonr]

CI job restarted: jenkins-ci/mbed-os-ci_greentea-test

Something weird happened with a target across multiple jobs.