Check connected flag before reconnecting

[vincentb1]

Without this check, the example_connect may receive spurious calls.

[sonarqubecloud]

Quality Gate failed

Failed conditions
100.0% Duplication on New Code (required ≤ 3%)

See analysis details on SonarQube Cloud

[joelguittet]

Hello @vincentb1 I would expect this to not occur with the example because the mender client is not requesting connection if already connected. Can you detail the error you get without this patch? Also please indicate your ESP-IDF version so that I can check.

Except this question, the modification looks good and provide a better network management so I don't see any reason not to do it. Can you add the same management on MENDER_CLIENT_EVENT_DISCONNECT event too? eg checking it is connected before disconnecting. This is done actually only when MENDER_CLIENT_EVENT_RESTART is received, but this can be ported to the normal use case (see my comment review in the source file).

[joelguittet]

To provide complete network management properly you can check here for connected flag so that network_disconnect is called only if we are already connected.

[vincentb1]

@joelguittet This can happen in corner cases : I had configured a short period for polling the server in order for my tests to go faster (a period only slightly longer than the download time), and the connection/deconnection not being instant I had some occurrence when the Mender client was trying to reconnect whereas some existing connection was still not fully released.

I think that adding this test does not eat any bread, so why not ? If you agree that it brings some value, then I propose that I provide some commission on top of it in order to factorize code and reduce if blocks imbrication depth (for code quality check to stop barking).

[joelguittet]

That's fine for me. Just to be clear: this application focus on mender integration, not spurious network events, but I'm fine to review and test improvements of course.
Please just notice I have not refactor because in case of restart requested, then I accept few connections again before restarting (so that we can notice of download status "restarting"). Also please note that when disconnection is requested, there is a 10 seconds delay before doing this, in case a connection is wanted again.

[joelguittet]

@vincentb1 hello
I'm actually working on pytest stuff to automate testing and it allowed me to reproduce your issue there. The log is "esp_netif_lwip: esp_netif_new_api: Failed to configure netif with config=0x3ffbae90 (config or if_key is NULL or duplicate key)", then a backtrace is shown and the system restarts.
Is it correct?
Do you want to finalize the PR here, according to the comments above, first by making an additional test on !connected before calling the disconnect function. Then optionally if you want to, in a separated commit, a refactor as you proposed.
Let me know, I'm happy to see your contribution here. Else it will be available soon when I will merge pytest stuff here.
Thanks
Joel

[sonarqubecloud]

Quality Gate passed

Issues
1 New issue
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
62.5% Duplication on New Code

See analysis details on SonarQube Cloud

[vincentb1]

Dear @joelguittet,
I have rebased the branch on master and inserted commit Test connected before calling example_disconnect..

Please note that I did not carry any tests (I just checked the project still compiles).

[joelguittet]

Superseded by #31