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

Receive long MQTT payload #1590

Merged
merged 3 commits into from Apr 8, 2021
Merged

Receive long MQTT payload #1590

merged 3 commits into from Apr 8, 2021

Conversation

gaco79
Copy link
Contributor

@gaco79 gaco79 commented Mar 6, 2021

What does this implement/fix?

MQTT fails when large message is delivered. Most noticeable with large JSON messages where JSON parsing then fails.

MQTT payload larger than TCP buffer will cause AsyncMQTTClient to split payload contents into separate parts.

The current ESPHome implementation assumes the whole payload will be delivered in one go

this->mqtt_client_.onMessage([this](char *topic, char *payload, AsyncMqttClientMessageProperties properties,
size_t len, size_t index, size_t total) {
std::string payload_s(payload, len);
std::string topic_s(topic);
this->on_message(topic_s, payload_s);
});

This can be fixed by moving the payload_s variable to a class variable and only calling MQTTClientComponent::on_message(topic_s, payload_s) when the full payload is available.

Types of changes

  • Bugfix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Configuration change (this will require users to update their yaml configuraiton files to keep working)

Test Environment

  • ESP32
  • ESP8266
  • Windows
  • Mac OS
  • Linux

Example entry for config.yaml:

# Example config.yaml - any standard MQTT setup will work 
mqtt:
  broker: !secret mqtt_broker
  username: !secret mqtt_username
  password: !secret mqtt_password
  on_json_message:
    - topic: test/data/
      then:
        logger.log: "JSON received"

Example long JSON message that causes error available here: https://pastebin.com/3DvKVeQR

Logs:

[18:37:06][D][main:174]: JSON received
[18:37:26][W][json:054]: Parsing JSON failed.
[18:37:26][W][json:054]: Parsing JSON failed.

Explain your changes

ESPHome can't be used if large MQTT messages are sent. This has presented a problem in my project where large MQTT payloads are sent containing colour information for strings of neopixel lights arranged in a panel. The only alternative has been to chunk the MQTT payloads into smaller sections, which in turn causes problems with not being able to update the whole "frame" of pixels at one go.

Checklist:

  • The code change is tested and works locally.
  • Tests have been added to verify that the new code works (under tests/ folder).

If user exposed functionality or configuration variables are added/changed:

esphome/components/mqtt/mqtt_client.cpp Outdated Show resolved Hide resolved
esphome/components/mqtt/mqtt_client.h Outdated Show resolved Hide resolved
esphome/components/mqtt/mqtt_client.cpp Outdated Show resolved Hide resolved
Copy link
Contributor

@blejdfist blejdfist left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me

Copy link
Member

@OttoWinter OttoWinter left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, left you one comment about the shrink_to_fit()

Copy link
Member

@OttoWinter OttoWinter left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks 🎉

@OttoWinter OttoWinter merged commit e6f8e73 into esphome:dev Apr 8, 2021
This was referenced May 9, 2021
loongyh added a commit to loongyh/esphome that referenced this pull request May 13, 2021
* Rewrite sun component calculations (esphome#1661)

* Raise minimum python version to 3.7 (esphome#1673)

* Fix colorlog removing colors and refactor color code (esphome#1671)

* Adds support for b-parasite soil moisture sensor (esphome#1666)

* mqtt_client: Added MQTTClientComponent::unsubscribe() (esphome#1672)

Co-authored-by: Otto Winter <otto@otto-winter.com>

* Fix servo detach chopped PWM (esphome#1650)

* Disallow _ in node name (esphome#1632)

* Receive long MQTT payload (esphome#1590)

* Fix sensor.sensor_schema interface changed (esphome#1659)

* Daylight Saving Time spelling fix (esphome#1677)

* Support custom build_flags for bme680_bsec (esphome#1678)

* Add Arduino ESP32 version mapping (esphome#1679)

See also https://github.com/platformio/platform-espressif32/releases/tag/v3.2.0

* Automate building and publishing of esphome-lint docker image (esphome#1680)

* Sgp40 (esphome#1513)

* Start of SGP40 dev

* Clean up

* Initial Commit

* VOC is working

* Fixed up sensor config

* Lint Fixes
Added in save/restore baseline
Noted original repo in header

* Lint Fixes
Added to test

* Lint Fixes

* Added additional check on restoring

* Removed double check

* Changed defines to static const double

* Changed defines to const
Do not send voc index until sensor stabilizes

* Fixed sensor stabilization message

* Fixup according to PR

* samples_read increment fix

* Fixed missing device class

* Choose a SENSOR device class

* Moved some sensors for tests

Co-authored-by: Guillermo Ruffino <glm.net@gmail.com>

* Bump protobuf from 3.15.7 to 3.15.8 (esphome#1682)

Bumps [protobuf](https://github.com/protocolbuffers/protobuf) from 3.15.7 to 3.15.8.
- [Release notes](https://github.com/protocolbuffers/protobuf/releases)
- [Changelog](https://github.com/protocolbuffers/protobuf/blob/master/generate_changelog.py)
- [Commits](protocolbuffers/protobuf@v3.15.7...v3.15.8)

Signed-off-by: dependabot[bot] <support@github.com>

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* Sensor Average Filter Fix Floating Pointer Error Accumulating (esphome#1624)

* Implementing the remainder of GPS data for the GPS component. (esphome#1676)

* Bump AsyncTCP-esphome to 1.2.1. (esphome#1693)

Co-authored-by: Maurice Makaay <mmakaay1@xs4all.net>

* Added / to default glyphs (esphome#1691)

Co-authored-by: Charlie Root <klingler@blender.klingler.net>

* Revert "Bump AsyncTCP-esphome to 1.2.1. (esphome#1693)" (esphome#1709)

This reverts commit aed6f2b.

* Change mac add mac suffix from underscore to dash (esphome#1702)

* GM40: Initial commit

* Add monochromatic effects: Pulse, Random (esphome#1616)

* Add support for SHT4X (esphome#1512)

* Addition of forward and reverse active energy counters to ATM90E32 sensor component (esphome#1271)

Co-authored-by: Elyor Khakimov <elyorkhakimov@forwardnetworks.com>

* Add Grow Fingerprint Reader (esphome#1356)

* RC522 fixes (esphome#1479)

* Support for TOF10120 distance sensor (esphome#1375)

* Swap fan and swing fields for Fujitu ACs (esphome#1635)

* Invert pos

* Change update() to callback. Invert GM40 set_pos.

* Fix invalid syntax

* GM40: Fix set_pos

* Rate limit update callback. Fix GM40 set_pos.

* send_update() only on ready_to_tx

* GM40: Add ±3% error margin to idle state

* GM40: Fix process_status()

* GM40: Fix process_status()(2)

* Linting fix

Co-authored-by: Otto Winter <otto@otto-winter.com>
Co-authored-by: rbaron <rbaron@rbaron.net>
Co-authored-by: Anthony Uk <1492471+dataway@users.noreply.github.com>
Co-authored-by: Guillermo Ruffino <glm.net@gmail.com>
Co-authored-by: Peter Kuehne <pkuehne@users.noreply.github.com>
Co-authored-by: Gareth Cooper <gareth@garethcooper.com>
Co-authored-by: Dan Gentry <dan@dashdrum.com>
Co-authored-by: SenexCrenshaw <35600301+SenexCrenshaw@users.noreply.github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: John Coggeshall <john@coggeshall.org>
Co-authored-by: Maurice Makaay <account+github@makaay.nl>
Co-authored-by: Maurice Makaay <mmakaay1@xs4all.net>
Co-authored-by: Richard Klingler <richard@klingler.net>
Co-authored-by: Charlie Root <klingler@blender.klingler.net>
Co-authored-by: Christian Ferbar <5595808+ferbar@users.noreply.github.com>
Co-authored-by: Stephen Tierney <sjtrny@gmail.com>
Co-authored-by: elyorkhakimov <elyor.khakimov@gmail.com>
Co-authored-by: Elyor Khakimov <elyorkhakimov@forwardnetworks.com>
Co-authored-by: Wojtek Strzalka <wstrzalka@users.noreply.github.com>
Co-authored-by: Alex <alexrichards.github@gmail.com>
loongyh pushed a commit to loongyh/esphome that referenced this pull request May 13, 2021
This was referenced May 18, 2021
@github-actions github-actions bot locked and limited conversation to collaborators Sep 14, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants