fix(mqtt5): Fix mqtt5 max inflight message counting

Split message validation path for mqtt5
Introduces tests on the conformance test suite

Fixes #312
This commit is contained in:
Euripedes Rocha Filho
2026-05-06 10:47:59 +02:00
parent 827875a087
commit 20b6eb0e30
12 changed files with 1198 additions and 155 deletions
+77 -48
View File
@@ -17,7 +17,7 @@ This app exposes a console API for pytest-embedded HIL tests that target MQTT co
## JSON config keys
All configuration is passed as a base64-encoded JSON object with exactly one top-level key, naming which config category the blob targets. The JSON shape mirrors the real esp_mqtt C struct layout, so field names/paths match `mqtt_client.h` / `mqtt5_client.h` directly.
All configuration is passed as a base64-encoded JSON object with a recognized top-level key naming the config category the blob targets. The JSON shape mirrors the real esp_mqtt C struct layout, so field names/paths match `mqtt_client.h` / `mqtt5_client.h` directly.
### `mqtt_config` (used with `init`)
@@ -25,63 +25,67 @@ Mirrors `esp_mqtt_client_config_t`'s nesting:
```json
{
"mqtt_config": {
"broker": { "address": { "uri": "mqtt://192.168.1.1:1883" } },
"credentials": { "client_id": "my-client" },
"session": { "keepalive": 30, "disable_clean_session": false, "protocol_ver": 3 },
"network": { "disable_auto_reconnect": true }
}
"mqtt_config": {
"broker": { "address": { "uri": "mqtt://192.168.1.1:1883" } },
"credentials": { "client_id": "my-client" },
"session": {
"keepalive": 30,
"disable_clean_session": false,
"protocol_ver": 3
},
"network": { "disable_auto_reconnect": true }
}
}
```
| Path | Type | Description |
|------|------|-------------|
| `broker.address.uri` | string | Broker URI (e.g. `mqtt://192.168.1.1:1883`) |
| `credentials.client_id` | string | Client identifier |
| `session.keepalive` | int | Keepalive interval (seconds) |
| `session.disable_clean_session` | bool | `true` = persistent session (clean start = false) |
| `session.protocol_ver` | int | Raw `esp_mqtt_protocol_ver_t` ordinal: `0`=UNDEFINED, `1`=MQTT 3.1, `2`=MQTT 3.1.1, `3`=MQTT 5.0 |
| `network.disable_auto_reconnect` | bool | Disable MQTT client automatic reconnect |
| Path | Type | Description |
| -------------------------------- | ------ | ------------------------------------------------------------------------------------------------ |
| `broker.address.uri` | string | Broker URI (e.g. `mqtt://192.168.1.1:1883`) |
| `credentials.client_id` | string | Client identifier |
| `session.keepalive` | int | Keepalive interval (seconds) |
| `session.disable_clean_session` | bool | `true` = persistent session (clean start = false) |
| `session.protocol_ver` | int | Raw `esp_mqtt_protocol_ver_t` ordinal: `0`=UNDEFINED, `1`=MQTT 3.1, `2`=MQTT 3.1.1, `3`=MQTT 5.0 |
| `network.disable_auto_reconnect` | bool | Disable MQTT client automatic reconnect |
### `connect_property` (MQTT5 connect properties)
Already flat in C, so the JSON object is flat too:
| Key | Type | Description |
|-----|------|-------------|
| `session_expiry_interval` | int | Session expiry (seconds) |
| `receive_maximum` | int | Receive maximum |
| `topic_alias_maximum` | int | Topic alias maximum |
| `maximum_packet_size` | int | Maximum packet size |
| `will_delay_interval` | int | Will delay interval (seconds) |
| Key | Type | Description |
| ------------------------- | ---- | ----------------------------- |
| `session_expiry_interval` | int | Session expiry (seconds) |
| `receive_maximum` | int | Receive maximum |
| `topic_alias_maximum` | int | Topic alias maximum |
| `maximum_packet_size` | int | Maximum packet size |
| `will_delay_interval` | int | Will delay interval (seconds) |
### `publish_property` (MQTT5 publish properties)
| Key | Type | Description |
|-----|------|-------------|
| `message_expiry_interval` | int | Message expiry (seconds) |
| `payload_format_indicator` | bool | `true` = UTF-8 encoded payload |
| `topic_alias` | int | Topic alias |
| `content_type` | string | Content type |
| `response_topic` | string | Response topic |
| Key | Type | Description |
| -------------------------- | ------ | ------------------------------ |
| `message_expiry_interval` | int | Message expiry (seconds) |
| `payload_format_indicator` | bool | `true` = UTF-8 encoded payload |
| `topic_alias` | int | Topic alias |
| `content_type` | string | Content type |
| `response_topic` | string | Response topic |
### `subscribe_property` (MQTT5 subscribe properties)
| Key | Type | Description |
|-----|------|-------------|
| `subscribe_id` | int | Subscription identifier |
| `no_local_flag` | bool | No local flag |
| `retain_as_published_flag` | bool | Retain as published flag |
| `retain_handle` | int | Retain handling option (0/1/2) |
| `is_share_subscribe` | bool | Shared subscription flag |
| `share_name` | string | Shared subscription group name |
| Key | Type | Description |
| -------------------------- | ------ | ------------------------------ |
| `subscribe_id` | int | Subscription identifier |
| `no_local_flag` | bool | No local flag |
| `retain_as_published_flag` | bool | Retain as published flag |
| `retain_handle` | int | Retain handling option (0/1/2) |
| `is_share_subscribe` | bool | Shared subscription flag |
| `share_name` | string | Shared subscription group name |
### `disconnect_property` (MQTT5 disconnect properties)
| Key | Type | Description |
|-----|------|-------------|
| `session_expiry_interval` | int | Session expiry override on disconnect |
| `disconnect_reason` | int | Disconnect reason code |
| Key | Type | Description |
| ------------------------- | ---- | ------------------------------------- |
| `session_expiry_interval` | int | Session expiry override on disconnect |
| `disconnect_reason` | int | Disconnect reason code |
## Conformance mapping
@@ -98,12 +102,37 @@ From the repository root (or the mqtt worktree root if using worktrees):
1. Ensure the environment is active (e.g. `direnv allow` at repo root so IDF and pytest-embedded are available).
2. Initialize the paho.mqtt.testing submodule:
```bash
git submodule update --init --recursive test/tools/paho.mqtt.testing
```
```bash
git submodule update --init --recursive test/tools/paho.mqtt.testing
```
3. Run the conformance tests (connect a board with Ethernet, or use the same target/port as in CI):
```bash
pytest test/apps/mqtt_conformance/ -v
```
To run a single test or filter by keyword, add e.g. `-k test_mqtt_v311` or the test path.
```bash
pytest test/apps/mqtt_conformance/ -v
```
To run a single test or filter by keyword, add e.g. `-k receive_maximum` or the test path.
## Optional environment variables
Each test starts its own fresh in-process paho broker on an OS-assigned ephemeral
port and tears it down at the end of that test, so brokers never carry state
between tests and there's no port to configure/coordinate.
- `MQTT_CONFORMANCE_PAHO_BROKER_LOG_LEVEL` — log level for the in-process paho broker's own
logger (default: `WARNING`).
- `MQTT_CONFORMANCE_HOST_IP` — host IPv4 address the DUT should use to reach the in-process
broker (default: auto-detected via a UDP socket connect to `8.8.8.8`).
- `MQTT_CONFORMANCE_CONNECT_RETRIES` — number of `start`/connect attempts before failing
(default: 3).
- `MQTT_CONFORMANCE_RETRY_BACKOFF_SEC` — backoff between connect retries, in seconds
(default: 2).
### Timeouts
Tests use **operation-based timeouts** (not a flat 60 s wait): the budget is computed
from the number of connect, subscribe, publish, and event-wait operations. Whole-test
ceilings use `@pytest.mark.timeout(...)`. Inflight tests do not rely on timing windows:
the broker explicitly holds and releases PUBACK or PUBCOMP packets around assertions.
@@ -410,7 +410,7 @@ void register_commands()
extern "C" void app_main(void)
{
constexpr size_t max_line = 512;
constexpr size_t max_line = 2048;
ESP_LOGI(TAG, "[APP] Free memory: %" PRIu32 " bytes", esp_get_free_heap_size());
ESP_LOGI(TAG, "[APP] IDF version: %s", esp_get_idf_version());
esp_log_level_set("*", ESP_LOG_INFO);
File diff suppressed because it is too large Load Diff
+1 -1
View File
@@ -1,4 +1,4 @@
idf_component_register(SRCS "test_mqtt_client.cpp" "test_log_intercept.cpp" "test_log_matchers.cpp" "test_log_parser.cpp"
idf_component_register(SRCS "test_mqtt_client.cpp" "test_mqtt5_client.cpp" "mqtt5_client_test_adapter.c" "test_log_intercept.cpp" "test_log_matchers.cpp" "test_log_parser.cpp"
REQUIRES cmock mqtt esp_timer esp_hw_support http_parser log
WHOLE_ARCHIVE)
@@ -0,0 +1,27 @@
/*
* SPDX-FileCopyrightText: 2026 Espressif Systems (Shanghai) CO LTD
*
* SPDX-License-Identifier: Apache-2.0
*/
#include <stdint.h>
#include "mqtt_client_priv.h"
esp_err_t test_mqtt5_check_inflight_maximum(uint16_t send_count, uint16_t receive_maximum)
{
struct esp_mqtt_client client = {0};
mqtt5_config_storage_t mqtt5_config = {0};
client.mqtt5_config = &mqtt5_config;
client.mqtt5_config->server_resp_property_info.receive_maximum = receive_maximum;
client.send_publish_packet_count = send_count;
return esp_mqtt5_client_check_inflight_maximum(&client);
}
int test_mqtt5_increment_packet_counter_with_dup(void)
{
struct esp_mqtt_client client = {0};
uint8_t publish_header[] = {0x3a}; // PUBLISH, DUP=1, QoS=1
client.mqtt_state.connection.outbound_message.data = publish_header;
esp_mqtt5_increment_packet_counter(&client);
return client.send_publish_packet_count;
}
+25
View File
@@ -0,0 +1,25 @@
/*
* SPDX-FileCopyrightText: 2026 Espressif Systems (Shanghai) CO LTD
*
* SPDX-License-Identifier: Apache-2.0
*/
#include <catch2/catch_test_macros.hpp>
#include <cstdint>
#include "esp_err.h"
extern "C" {
esp_err_t test_mqtt5_check_inflight_maximum(uint16_t send_count, uint16_t receive_maximum);
int test_mqtt5_increment_packet_counter_with_dup(void);
}
TEST_CASE("MQTT5 inflight quota uses an exact upper bound")
{
REQUIRE(test_mqtt5_check_inflight_maximum(1, 2) == ESP_OK);
REQUIRE(test_mqtt5_check_inflight_maximum(2, 2) == ESP_FAIL);
}
TEST_CASE("MQTT5 first send on a connection counts even when PUBLISH has DUP set")
{
REQUIRE(test_mqtt5_increment_packet_counter_with_dup() == 1);
}
+1
View File
@@ -1,4 +1,5 @@
CONFIG_IDF_TARGET="linux"
CONFIG_MQTT_PROTOCOL_5=y
CONFIG_LOG_DEFAULT_LEVEL_DEBUG=y
CONFIG_COMPILER_CXX_EXCEPTIONS=y
CONFIG_COMPILER_CXX_RTTI=y
@@ -204,6 +204,31 @@ TEST_CASE("Outbox lookup by msg_id")
outbox_enqueue(outbox.handle, &message, 0);
REQUIRE(outbox_get(outbox.handle, 999) == nullptr);
}
SECTION("msg_id zero finds queued QoS 0 behind a QoS 1 head") {
auto qos1 = make_msg(1, 1, 3, "qos1", 4);
auto qos0_first = make_msg(0, 0, 3, "first", 5);
auto qos0_second = make_msg(0, 0, 3, "second", 6);
outbox_enqueue(outbox.handle, &qos1, 0);
outbox_enqueue(outbox.handle, &qos0_first, 0);
outbox_enqueue(outbox.handle, &qos0_second, 0);
REQUIRE(outbox_dequeue(outbox.handle, QUEUED, nullptr) == outbox_get(outbox.handle, 1));
outbox_item_handle_t item = outbox_get(outbox.handle, 0);
REQUIRE(item != nullptr);
REQUIRE(outbox_item_get_pending(item) == QUEUED);
uint16_t id;
int type, qos;
size_t len;
auto *data = outbox_item_get_data(item, &len, &id, &type, &qos);
REQUIRE(id == 0);
REQUIRE(type == 3);
REQUIRE(qos == 0);
REQUIRE(std::string(reinterpret_cast<char *>(data), len) == "first");
REQUIRE(outbox_delete_item(outbox.handle, item) == ESP_OK);
item = outbox_get(outbox.handle, 0);
REQUIRE(item != nullptr);
data = outbox_item_get_data(item, &len, &id, &type, &qos);
REQUIRE(std::string(reinterpret_cast<char *>(data), len) == "second");
}
}
TEST_CASE("Outbox delete by msg_id and type")