Skip to content

Commit

Permalink
Subscribing to messages using Homie.getMqttClient() is not reliable: …
Browse files Browse the repository at this point in the history
…topic is truncated (#526)

* Declare _mqttTopicCopy in BootNormal.hpp

* Using _mqttTopicCopy to manage subscribed topics

* Removed leading spaces
  • Loading branch information
peret2000 authored and luebbe committed Nov 12, 2019
1 parent 330838f commit a055cd1
Show file tree
Hide file tree
Showing 2 changed files with 16 additions and 9 deletions.
24 changes: 15 additions & 9 deletions src/Homie/Boot/BootNormal.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,9 @@ BootNormal::BootNormal()
, _mqttWillTopic(nullptr)
, _mqttPayloadBuffer(nullptr)
, _mqttTopicLevels(nullptr)
, _mqttTopicLevelsCount(0) {
strlcpy(_fwChecksum, ESP.getSketchMD5().c_str(), sizeof(_fwChecksum));
, _mqttTopicLevelsCount(0)
, _mqttTopicCopy(nullptr)
{strlcpy(_fwChecksum, ESP.getSketchMD5().c_str(), sizeof(_fwChecksum));
_fwChecksum[sizeof(_fwChecksum) - 1] = '\0';
}

Expand Down Expand Up @@ -524,39 +525,44 @@ void BootNormal::_onMqttDisconnected(AsyncMqttClientDisconnectReason reason) {
void BootNormal::_onMqttMessage(char* topic, char* payload, AsyncMqttClientMessageProperties properties, size_t len, size_t index, size_t total) {
if (total == 0) return; // no empty message possible

size_t topicLength = strlen(topic);
_mqttTopicCopy = std::unique_ptr<char[]>(new char[topicLength+1]);
memcpy(_mqttTopicCopy.get(), topic, topicLength);
_mqttTopicCopy.get()[topicLength] = '\0';

// split topic on each "/"
if (index == 0) {
__splitTopic(topic);
__splitTopic(_mqttTopicCopy.get());
}

// 1. Handle OTA firmware (not copied to payload buffer)
if (__handleOTAUpdates(topic, payload, properties, len, index, total))
if (__handleOTAUpdates(_mqttTopicCopy.get(), payload, properties, len, index, total))
return;

// 2. Fill Payload Buffer
if (__fillPayloadBuffer(topic, payload, properties, len, index, total))
if (__fillPayloadBuffer(_mqttTopicCopy.get(), payload, properties, len, index, total))
return;

/* Arrived here, the payload is complete */

// 3. handle broadcasts
if (__handleBroadcasts(topic, payload, properties, len, index, total))
if (__handleBroadcasts(_mqttTopicCopy.get(), payload, properties, len, index, total))
return;

// 4.all following messages are only for this deviceId
if (strcmp(_mqttTopicLevels.get()[0], Interface::get().getConfig().get().deviceId) != 0)
return;

// 5. handle reset
if (__handleResets(topic, payload, properties, len, index, total))
if (__handleResets(_mqttTopicCopy.get(), payload, properties, len, index, total))
return;

// 6. handle config set
if (__handleConfig(topic, payload, properties, len, index, total))
if (__handleConfig(_mqttTopicCopy.get(), payload, properties, len, index, total))
return;

// 7. here, we're sure we have a node property
if (__handleNodeProperty(topic, payload, properties, len, index, total))
if (__handleNodeProperty(_mqttTopicCopy.get(), payload, properties, len, index, total))
return;
}

Expand Down
1 change: 1 addition & 0 deletions src/Homie/Boot/BootNormal.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,7 @@ class BootNormal : public Boot {
std::unique_ptr<char[]> _mqttPayloadBuffer;
std::unique_ptr<char*[]> _mqttTopicLevels;
uint8_t _mqttTopicLevelsCount;
std::unique_ptr<char[]> _mqttTopicCopy;

void _wifiConnect();
void _onWifiGotIp(const WiFiEventStationModeGotIP& event);
Expand Down

2 comments on commit a055cd1

@luebbe
Copy link
Collaborator

@luebbe luebbe commented on a055cd1 Nov 12, 2019

Choose a reason for hiding this comment

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

I merged this PR into develop, because it looks ok, but I'm at a loss how to get it into the develop-v3 branch.

@kleini
Copy link
Collaborator

@kleini kleini commented on a055cd1 Nov 12, 2019

Choose a reason for hiding this comment

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

Create another merge request with different target or cherry-pick the commits to the develop-v3 branch.

Please sign in to comment.