-
Notifications
You must be signed in to change notification settings - Fork 7
Enhancements to the pub sub client reconnect unsubscribe publish with time out #128
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
base: main
Are you sure you want to change the base?
Changes from 9 commits
e56fb05
cda238f
6e477c9
85a4849
217b50c
629284f
ad99c59
d79c87f
8b6f624
5ce288b
d7b72d6
d35ca22
204227b
0e179be
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,73 @@ | ||
{ | ||
"files.associations": { | ||
"array": "cpp", | ||
"atomic": "cpp", | ||
"bit": "cpp", | ||
"*.tcc": "cpp", | ||
"cctype": "cpp", | ||
"chrono": "cpp", | ||
"clocale": "cpp", | ||
"cmath": "cpp", | ||
"compare": "cpp", | ||
"concepts": "cpp", | ||
"condition_variable": "cpp", | ||
"csignal": "cpp", | ||
"cstdint": "cpp", | ||
"cstdio": "cpp", | ||
"cstdlib": "cpp", | ||
"cstring": "cpp", | ||
"ctime": "cpp", | ||
"cwchar": "cpp", | ||
"cwctype": "cpp", | ||
"deque": "cpp", | ||
"list": "cpp", | ||
"map": "cpp", | ||
"unordered_map": "cpp", | ||
"vector": "cpp", | ||
"exception": "cpp", | ||
"functional": "cpp", | ||
"initializer_list": "cpp", | ||
"iosfwd": "cpp", | ||
"iostream": "cpp", | ||
"istream": "cpp", | ||
"limits": "cpp", | ||
"memory": "cpp", | ||
"mutex": "cpp", | ||
"new": "cpp", | ||
"numbers": "cpp", | ||
"ostream": "cpp", | ||
"ratio": "cpp", | ||
"semaphore": "cpp", | ||
"stdexcept": "cpp", | ||
"stop_token": "cpp", | ||
"streambuf": "cpp", | ||
"string": "cpp", | ||
"string_view": "cpp", | ||
"system_error": "cpp", | ||
"thread": "cpp", | ||
"tuple": "cpp", | ||
"type_traits": "cpp", | ||
"typeinfo": "cpp", | ||
"utility": "cpp", | ||
"any": "cpp", | ||
"codecvt": "cpp", | ||
"cstdarg": "cpp", | ||
"cstddef": "cpp", | ||
"forward_list": "cpp", | ||
"set": "cpp", | ||
"unordered_set": "cpp", | ||
"algorithm": "cpp", | ||
"iterator": "cpp", | ||
"memory_resource": "cpp", | ||
"numeric": "cpp", | ||
"optional": "cpp", | ||
"random": "cpp", | ||
"fstream": "cpp", | ||
"iomanip": "cpp", | ||
"shared_mutex": "cpp", | ||
"sstream": "cpp", | ||
"cinttypes": "cpp", | ||
"variant": "cpp", | ||
"future": "cpp" | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -59,6 +59,15 @@ struct VoidResult {}; | |
|
||
enum class CallState { ONGOING, CANCELING, COMPLETED, FAILED }; | ||
|
||
/** | ||
* @brief Status of an MQTT publish operation | ||
moamenvx marked this conversation as resolved.
Show resolved
Hide resolved
|
||
*/ | ||
enum PublishStatus { | ||
Success, // Message was published successfully | ||
Timeout, // Publish operation timed out | ||
Failure // Publish operation failed (e.g., exception thrown) | ||
}; | ||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Not used here. Should be moved to |
||
/** | ||
* @brief Single result of an asynchronous operation which provides | ||
* an item of type TResultType. | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -104,6 +104,13 @@ class IPubSubClient { | |
*/ | ||
virtual void connect() = 0; | ||
|
||
/** | ||
* @brief Reconnect the client to the broker. | ||
* @param timeout_ms maximum time to wait for the reconnection attempt to complete, in | ||
* milliseconds. | ||
*/ | ||
virtual void reconnect(int timeout_ms) = 0; | ||
|
||
Comment on lines
+107
to
+113
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This should not be visible to the app "user" code: Instead, reconnection should be done "under the hood" within the PubSubClient after calling |
||
/** | ||
* @brief Disconnect the client from the broker. | ||
* | ||
|
@@ -126,6 +133,20 @@ class IPubSubClient { | |
*/ | ||
virtual void publishOnTopic(const std::string& topic, const std::string& data) = 0; | ||
|
||
/** | ||
* @brief Publishes a message to the specified MQTT topic with a timeout in milliseconds for the | ||
* publish to complete. Returns a status indicating whether the publish was successful, timed | ||
* out, or failed. | ||
* | ||
* @param topic the MQTT topic to publish the message to | ||
* @param data the payload to send as the message | ||
* @param timeout_ms maximum time to wait for the publish to complete, in milliseconds | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please document the corner cases: What does a timeout of 0 ms mean? What about negative numbers? |
||
* @return PublishStatus indicating the result of the publish operation: Success, Timeout, | ||
* Failure | ||
*/ | ||
virtual PublishStatus publishOnTopic(const std::string& topic, const std::string& data, | ||
int timeout_ms) = 0; | ||
|
||
/** | ||
* @brief Subscribe to a topic. | ||
* | ||
|
@@ -134,6 +155,13 @@ class IPubSubClient { | |
*/ | ||
virtual AsyncSubscriptionPtr_t<std::string> subscribeTopic(const std::string& topic) = 0; | ||
|
||
/** | ||
* @brief Unsubscribe from a topic. | ||
* | ||
* @param topic The topic to unsubscribe from. | ||
*/ | ||
virtual void unsubscribeTopic(const std::string& topic) = 0; | ||
|
||
IPubSubClient(const IPubSubClient&) = delete; | ||
IPubSubClient(IPubSubClient&&) = delete; | ||
IPubSubClient& operator=(const IPubSubClient&) = delete; | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -22,6 +22,7 @@ | |
#include "sdk/middleware/Middleware.h" | ||
|
||
#include <mqtt/async_client.h> | ||
#include <future> | ||
#include <mqtt/connect_options.h> | ||
#include <unordered_map> | ||
|
||
|
@@ -60,14 +61,13 @@ class MqttPubSubClient : public IPubSubClient, private mqtt::callback { | |
const std::string& privateKeyPath) | ||
: m_client{brokerUri, clientId} { | ||
m_client.set_callback(*this); | ||
auto sslopts = mqtt::ssl_options_builder() | ||
.trust_store(trustStorePath) | ||
.key_store(keyStorePath) | ||
.private_key(privateKeyPath) | ||
.error_handler([](const std::string& msg) { | ||
logger().error("SSL Error: {}", msg); | ||
}) | ||
.finalize(); | ||
auto sslopts = | ||
mqtt::ssl_options_builder() | ||
.trust_store(trustStorePath) | ||
.key_store(keyStorePath) | ||
.private_key(privateKeyPath) | ||
.error_handler([](const std::string& msg) { logger().error("SSL Error: {}", msg); }) | ||
.finalize(); | ||
m_connectOptions = mqtt::connect_options_builder().ssl(std::move(sslopts)).finalize(); | ||
} | ||
|
||
|
@@ -85,6 +85,22 @@ class MqttPubSubClient : public IPubSubClient, private mqtt::callback { | |
|
||
m_client.connect(m_connectOptions)->wait(); | ||
} | ||
|
||
void reconnect(int timeout_ms) override { | ||
logger().info("Attempting to reconnect to MQTT broker"); | ||
|
||
try { | ||
auto token = m_client.reconnect(); | ||
if (!token->wait_for(std::chrono::milliseconds(timeout_ms))) { | ||
logger().error("MQTT reconnect timed out after {} ms", timeout_ms); | ||
} else { | ||
logger().info("Successfully reconnected to MQTT broker."); | ||
} | ||
} catch (const mqtt::exception& ex) { | ||
logger().error("MQTT reconnect failed: {}", ex.what()); | ||
} | ||
} | ||
|
||
void disconnect() override { m_client.disconnect()->wait(); } | ||
[[nodiscard]] bool isConnected() const override { return m_client.is_connected(); } | ||
|
||
|
@@ -93,6 +109,36 @@ class MqttPubSubClient : public IPubSubClient, private mqtt::callback { | |
m_client.publish(topic, data)->wait(); | ||
} | ||
|
||
PublishStatus publishOnTopic(const std::string& topic, const std::string& data, | ||
int timeout_ms) override { | ||
try { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I know the existing function is block-waiting, but for this new function it could make sense to return a future or a AsyncResult to let the user decide if they like to do blocking wait or will use Callbacks for async notification (or likes to ignore outcome at all). In case of keeping the blocking variant, you should think about using Paho mqtt's waitFor() ... |
||
logger().debug(R"(Publish on topic "{}": "{}")", topic, data); | ||
|
||
auto future = std::async(std::launch::async, [this, &topic, &data]() { | ||
auto tok = m_client.publish(topic, data); | ||
if (!tok) { | ||
throw mqtt::exception(MQTTASYNC_FAILURE); | ||
} | ||
tok->wait(); | ||
return PublishStatus::Success; | ||
}); | ||
|
||
if (future.wait_for(std::chrono::milliseconds(timeout_ms)) == | ||
std::future_status::ready) { | ||
return future.get(); // Success | ||
} else { | ||
logger().warn("Publish timed out after {} ms", timeout_ms); | ||
return PublishStatus::Timeout; | ||
} | ||
} catch (const mqtt::exception& ex) { | ||
logger().error("MQTT publish failed: {}", ex.what()); | ||
return PublishStatus::Failure; | ||
} catch (const std::exception& ex) { | ||
logger().error("Unexpected exception during publish: {}", ex.what()); | ||
return PublishStatus::Failure; | ||
} | ||
} | ||
|
||
AsyncSubscriptionPtr_t<std::string> subscribeTopic(const std::string& topic) override { | ||
logger().debug("Subscribing to {}", topic); | ||
auto subscription = std::make_shared<AsyncSubscription<std::string>>(); | ||
|
@@ -101,6 +147,13 @@ class MqttPubSubClient : public IPubSubClient, private mqtt::callback { | |
return subscription; | ||
} | ||
|
||
void unsubscribeTopic(const std::string& topic) override { | ||
logger().debug("Unsubscribing from {}", topic); | ||
m_client.unsubscribe(topic)->wait(); | ||
auto range = m_subscriberMap.equal_range(topic); | ||
m_subscriberMap.erase(range.first, range.second); | ||
} | ||
|
||
private: | ||
void message_arrived(mqtt::const_message_ptr msg) override { | ||
const std::string& topic = msg->get_topic(); | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,46 @@ | ||
/** | ||
* Copyright (c) 2022-2025 Contributors to the Eclipse Foundation | ||
* | ||
* This program and the accompanying materials are made available under the | ||
* terms of the Apache License, Version 2.0 which is available at | ||
* https://www.apache.org/licenses/LICENSE-2.0. | ||
* | ||
* Unless required by applicable law or agreed to in writing, software | ||
* distributed under the License is distributed on an "AS IS" BASIS, WITHOUT | ||
* WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the | ||
* License for the specific language governing permissions and limitations | ||
* under the License. | ||
* | ||
* SPDX-License-Identifier: Apache-2.0 | ||
*/ | ||
|
||
#ifndef MOCK_IPUBSUBCLIENT_H | ||
#define MOCK_IPUBSUBCLIENT_H | ||
|
||
#include "sdk/IPubSubClient.h" | ||
#include <gmock/gmock.h> | ||
|
||
namespace velocitas { | ||
|
||
class MockIPubSubClient : public IPubSubClient { | ||
public: | ||
MOCK_METHOD(void, connect, (), (override)); | ||
MOCK_METHOD(void, disconnect, (), (override)); | ||
MOCK_METHOD(void, reconnect, (int timeout_ms), (override)); | ||
MOCK_METHOD(bool, isConnected, (), (const, override)); | ||
|
||
MOCK_METHOD(void, publishOnTopic, (const std::string& topic, const std::string& data), | ||
(override)); | ||
|
||
MOCK_METHOD(PublishStatus, publishOnTopic, | ||
(const std::string& topic, const std::string& data, int timeout_ms), (override)); | ||
|
||
MOCK_METHOD(AsyncSubscriptionPtr_t<std::string>, subscribeTopic, (const std::string& topic), | ||
(override)); | ||
|
||
MOCK_METHOD(void, unsubscribeTopic, (const std::string& topic), (override)); | ||
}; | ||
|
||
} // namespace velocitas | ||
|
||
#endif // MOCK_IPUBSUBCLIENT_H |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please fix this merge conflict
You can get the required content just from the output of the failing workflow. There is a section where you can copy the content from. Just past it over the existing content of this file. Make sure there is exactly one single FF after the last license entry!