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

reconnect not callable from event (IDFGH-5918) #206

Closed
bertmelis opened this issue Sep 27, 2021 · 3 comments
Closed

reconnect not callable from event (IDFGH-5918) #206

bertmelis opened this issue Sep 27, 2021 · 3 comments

Comments

@bertmelis
Copy link
Contributor

I have autoreconnect disabled.

Am I correct that you cannot call esp_mqtt_client_reconnect from within the event handler on MQTT_EVENT_DISCONNECTED events? I get a ESP_FAIL error because the clients isn't ready yet.

Is this intentional?

@github-actions github-actions bot changed the title reconnect not callable from event reconnect not callable from event (IDFGH-5918) Sep 27, 2021
@david-cermak
Copy link
Collaborator

@bertmelis Thanks for the report, this is definitely a faulty behavior.

Is this intentional?

It was intentional in the past, as disable_auto_reconnect was introduced after manual reconnection, so it was supposed to work properly, only if autoreconnect been disabled by setting the reconnect_timeout_ms a very-very big number. But now we have to update the reconnection logic.

The WIP fix is below:
Client-Fix-reconnect-disconnect-if-auto_reconnect-of.patch.txt

This works for me in this connect-disconnect scenario:

    switch (event_id) {
    case MQTT_EVENT_CONNECTED:
        esp_mqtt_client_disconnect(client);
        break;
    case MQTT_EVENT_DISCONNECTED:
        esp_mqtt_client_reconnect(client);
        break;
    default:
        break;
    }

We're planning on adding more unit tests to cover these basic state transitions to avoid happening this in future.

@bertmelis
Copy link
Contributor Author

Thanks. I'll try with the patch.

Just a question: are unit tests you're talking about public? In this repo CI is only to check whether the example builds, right?

@david-cermak
Copy link
Collaborator

david-cermak commented Oct 3, 2021

are unit tests you're talking about public?

Most of the mqtt tests are located in IDF, but, yes, they are public. As for the unit tests we have a very simple suite of

In this repo CI is only to check whether the example builds

This GitHub repo runs a very basic travis build only. In addition to that we run a simple CI pipeline on the internal mirror, basically building against multiple IDF versions and running a smoke test checking for common connection/publishing scenarios:

esp-mqtt/.gitlab-ci.yml

Lines 120 to 122 in 89894bd

# run test (with environment->qemu)
- cd $IDF_PATH/tools/ci/python_packages/tiny_test_fw/bin
- python Runner.py $TEST_PATH -c $TEST_PATH/publish_connect_mqtt_qemu.yml -e $TEST_PATH/env.yml

(again running an IDF test app )

espressif-bot pushed a commit to espressif/esp-idf that referenced this issue Dec 3, 2021
Updated MQTT submodule: git log --oneline 89894bd0c611b1392967fe90bb49682eba858383...b86d42c130ac64a916ce6cf299d99f9756692394
* Added support for client with empty id
* Fixed user requested disconnect to correctly send MQTT disconnection message
* Fixed reconnection request with disabled autoreconnect
* Added qos and dup flags to data events
* Added Support for suback massage payload in mqtt events

Detailed description of the changes (espressif/esp-mqtt@89894bd...b86d42c):
* Adds the possibility of client with empty id
    - See merge request esp-mqtt!114
    - esp_mqtt commit espressif/esp-mqtt@09287a1
    - esp_mqtt commit espressif/esp-mqtt@1fd50dd
    - Related IDF-4124
* Client: Disconnect/Reconnect improvements
    - See merge request esp-mqtt!113
    - esp_mqtt commit espressif/esp-mqtt@3f05b1a
    - esp_mqtt commit espressif/esp-mqtt@86e40f8
    - Related espressif/esp-mqtt#206
    - Related espressif/esp-mqtt#208
* Events: Support qos/dup flags and suback payload in mqtt events (GitHub PR)
    - See merge request esp-mqtt!112
    - esp_mqtt commit espressif/esp-mqtt@de47f1c
    - esp_mqtt commit espressif/esp-mqtt@e1d5a94
    - Related espressif/esp-mqtt#200
    - Related espressif/esp-mqtt#203
david-cermak added a commit that referenced this issue Dec 16, 2022
Updated MQTT submodule: git log --oneline 89894bd...b86d42c
* Added support for client with empty id
* Fixed user requested disconnect to correctly send MQTT disconnection message
* Fixed reconnection request with disabled autoreconnect
* Added qos and dup flags to data events
* Added Support for suback massage payload in mqtt events

Detailed description of the changes (89894bd...b86d42c):
* Adds the possibility of client with empty id
    - See merge request esp-mqtt!114
    - esp_mqtt commit 09287a1
    - esp_mqtt commit 1fd50dd
    - Related IDF-4124
* Client: Disconnect/Reconnect improvements
    - See merge request esp-mqtt!113
    - esp_mqtt commit 3f05b1a
    - esp_mqtt commit 86e40f8
    - Related #206
    - Related #208
* Events: Support qos/dup flags and suback payload in mqtt events (GitHub PR)
    - See merge request esp-mqtt!112
    - esp_mqtt commit de47f1c
    - esp_mqtt commit e1d5a94
    - Related #200
    - Related #203
egnor pushed a commit to egnor/esp-mqtt that referenced this issue Dec 23, 2022
Updated MQTT submodule: git log --oneline 89894bd...b86d42c
* Added support for client with empty id
* Fixed user requested disconnect to correctly send MQTT disconnection message
* Fixed reconnection request with disabled autoreconnect
* Added qos and dup flags to data events
* Added Support for suback massage payload in mqtt events

Detailed description of the changes (espressif/esp-mqtt@89894bd...b86d42c):
* Adds the possibility of client with empty id
    - See merge request esp-mqtt!114
    - esp_mqtt commit espressif@09287a1
    - esp_mqtt commit espressif@1fd50dd
    - Related IDF-4124
* Client: Disconnect/Reconnect improvements
    - See merge request esp-mqtt!113
    - esp_mqtt commit espressif@3f05b1a
    - esp_mqtt commit espressif@86e40f8
    - Related espressif#206
    - Related espressif#208
* Events: Support qos/dup flags and suback payload in mqtt events (GitHub PR)
    - See merge request esp-mqtt!112
    - esp_mqtt commit espressif@de47f1c
    - esp_mqtt commit espressif@e1d5a94
    - Related espressif#200
    - Related espressif#203
egnor pushed a commit to egnor/esp-mqtt that referenced this issue Dec 23, 2022
Updated MQTT submodule: git log --oneline 89894bd...b86d42c
* Added support for client with empty id
* Fixed user requested disconnect to correctly send MQTT disconnection message
* Fixed reconnection request with disabled autoreconnect
* Added qos and dup flags to data events
* Added Support for suback massage payload in mqtt events

Detailed description of the changes (espressif/esp-mqtt@89894bd...b86d42c):
* Adds the possibility of client with empty id
    - See merge request esp-mqtt!114
    - esp_mqtt commit espressif@09287a1
    - esp_mqtt commit espressif@1fd50dd
    - Related IDF-4124
* Client: Disconnect/Reconnect improvements
    - See merge request esp-mqtt!113
    - esp_mqtt commit espressif@3f05b1a
    - esp_mqtt commit espressif@86e40f8
    - Related espressif#206
    - Related espressif#208
* Events: Support qos/dup flags and suback payload in mqtt events (GitHub PR)
    - See merge request esp-mqtt!112
    - esp_mqtt commit espressif@de47f1c
    - esp_mqtt commit espressif@e1d5a94
    - Related espressif#200
    - Related espressif#203
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

No branches or pull requests

2 participants