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

Fixes #391: Don't use mqtt::optional<> for the suback handler, as the standard requires the values be provided #434

Merged
merged 1 commit into from
Sep 15, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 2 additions & 7 deletions example/no_tls_both.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -77,15 +77,10 @@ void client_proc(
});
c->set_suback_handler(
[&]
(packet_id_t packet_id, std::vector<MQTT_NS::optional<MQTT_NS::suback_reason_code>> results){
(packet_id_t packet_id, std::vector<MQTT_NS::suback_reason_code> results){
std::cout << "[client] suback received. packet_id: " << packet_id << std::endl;
for (auto const& e : results) {
if (e) {
std::cout << "[client] subscribe success: " << static_cast<MQTT_NS::qos>(*e) << std::endl;
}
else {
std::cout << "[client] subscribe failed" << std::endl;
}
std::cout << "[client] subscribe result: " << e << std::endl;
}
if (packet_id == pid_sub1) {
c->publish("mqtt_client_cpp/topic1", "test1", MQTT_NS::qos::at_most_once);
Expand Down
9 changes: 2 additions & 7 deletions example/no_tls_client.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -85,15 +85,10 @@ int main(int argc, char** argv) {
});
c->set_suback_handler(
[&]
(packet_id_t packet_id, std::vector<MQTT_NS::optional<MQTT_NS::suback_reason_code>> results){
(packet_id_t packet_id, std::vector<MQTT_NS::suback_reason_code> results){
std::cout << "suback received. packet_id: " << packet_id << std::endl;
for (auto const& e : results) {
if (e) {
std::cout << "subscribe success: " << static_cast<MQTT_NS::qos>(*e) << std::endl;
}
else {
std::cout << "subscribe failed" << std::endl;
}
std::cout << "[client] subscribe result: " << e << std::endl;
}
if (packet_id == pid_sub1) {
c->publish("mqtt_client_cpp/topic1", "test1", MQTT_NS::qos::at_most_once);
Expand Down
9 changes: 2 additions & 7 deletions example/no_tls_ws_both.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -77,15 +77,10 @@ void client_proc(
});
c->set_suback_handler(
[&]
(packet_id_t packet_id, std::vector<MQTT_NS::optional<MQTT_NS::suback_reason_code>> results){
(packet_id_t packet_id, std::vector<MQTT_NS::suback_reason_code> results){
std::cout << "[client] suback received. packet_id: " << packet_id << std::endl;
for (auto const& e : results) {
if (e) {
std::cout << "[client] subscribe success: " << static_cast<MQTT_NS::qos>(*e) << std::endl;
}
else {
std::cout << "[client] subscribe failed" << std::endl;
}
std::cout << "[client] subscribe result: " << e << std::endl;
}
if (packet_id == pid_sub1) {
c->publish("mqtt_client_cpp/topic1", "test1", MQTT_NS::qos::at_most_once);
Expand Down
9 changes: 2 additions & 7 deletions example/no_tls_ws_client.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -85,15 +85,10 @@ int main(int argc, char** argv) {
});
c->set_suback_handler(
[&]
(packet_id_t packet_id, std::vector<MQTT_NS::optional<MQTT_NS::suback_reason_code>> results){
(packet_id_t packet_id, std::vector<MQTT_NS::suback_reason_code> results){
std::cout << "suback received. packet_id: " << packet_id << std::endl;
for (auto const& e : results) {
if (e) {
std::cout << "subscribe success: " << static_cast<MQTT_NS::qos>(*e) << std::endl;
}
else {
std::cout << "subscribe failed" << std::endl;
}
std::cout << "[client] subscribe result: " << e << std::endl;
}
if (packet_id == pid_sub1) {
c->publish("mqtt_client_cpp/topic1", "test1", MQTT_NS::qos::at_most_once);
Expand Down
9 changes: 2 additions & 7 deletions example/tls_both.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -78,15 +78,10 @@ void client_proc(
});
c->set_suback_handler(
[&]
(packet_id_t packet_id, std::vector<MQTT_NS::optional<MQTT_NS::suback_reason_code>> results){
(packet_id_t packet_id, std::vector<MQTT_NS::suback_reason_code> results){
std::cout << "[client] suback received. packet_id: " << packet_id << std::endl;
for (auto const& e : results) {
if (e) {
std::cout << "[client] subscribe success: " << static_cast<MQTT_NS::qos>(*e) << std::endl;
}
else {
std::cout << "[client] subscribe failed" << std::endl;
}
std::cout << "[client] subscribe result: " << e << std::endl;
}
if (packet_id == pid_sub1) {
c->publish("mqtt_client_cpp/topic1", "test1", MQTT_NS::qos::at_most_once);
Expand Down
9 changes: 2 additions & 7 deletions example/tls_client.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -100,15 +100,10 @@ int main(int argc, char** argv) {
});
c->set_suback_handler(
[&]
(packet_id_t packet_id, std::vector<MQTT_NS::optional<MQTT_NS::suback_reason_code>> results){
(packet_id_t packet_id, std::vector<MQTT_NS::suback_reason_code> results){
std::cout << "suback received. packet_id: " << packet_id << std::endl;
for (auto const& e : results) {
if (e) {
std::cout << "subscribe success: " << static_cast<MQTT_NS::qos>(*e) << std::endl;
}
else {
std::cout << "subscribe failed" << std::endl;
}
std::cout << "[client] subscribe result: " << e << std::endl;
}
if (packet_id == pid_sub1) {
c->publish("mqtt_client_cpp/topic1", "test1", MQTT_NS::qos::at_most_once);
Expand Down
9 changes: 2 additions & 7 deletions example/tls_ws_both.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -77,15 +77,10 @@ void client_proc(
});
c->set_suback_handler(
[&]
(packet_id_t packet_id, std::vector<MQTT_NS::optional<MQTT_NS::suback_reason_code>> results){
(packet_id_t packet_id, std::vector<MQTT_NS::suback_reason_code> results){
std::cout << "[client] suback received. packet_id: " << packet_id << std::endl;
for (auto const& e : results) {
if (e) {
std::cout << "[client] subscribe success: " << static_cast<MQTT_NS::qos>(*e) << std::endl;
}
else {
std::cout << "[client] subscribe failed" << std::endl;
}
std::cout << "[client] subscribe result: " << e << std::endl;
}
if (packet_id == pid_sub1) {
c->publish("mqtt_client_cpp/topic1", "test1", MQTT_NS::qos::at_most_once);
Expand Down
9 changes: 2 additions & 7 deletions example/tls_ws_client.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -91,15 +91,10 @@ int main(int argc, char** argv) {
});
c->set_suback_handler(
[&]
(packet_id_t packet_id, std::vector<MQTT_NS::optional<MQTT_NS::suback_reason_code>> results){
(packet_id_t packet_id, std::vector<MQTT_NS::suback_reason_code> results){
std::cout << "suback received. packet_id: " << packet_id << std::endl;
for (auto const& e : results) {
if (e) {
std::cout << "subscribe success: " << static_cast<MQTT_NS::qos>(*e) << std::endl;
}
else {
std::cout << "subscribe failed" << std::endl;
}
std::cout << "[client] subscribe result: " << e << std::endl;
}
if (packet_id == pid_sub1) {
c->publish("mqtt_client_cpp/topic1", "test1", MQTT_NS::qos::at_most_once);
Expand Down
32 changes: 23 additions & 9 deletions include/mqtt/endpoint.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -282,7 +282,7 @@ class endpoint : public std::enable_shared_from_this<endpoint<Mutex, LockGuard,
* @return if the handler returns true, then continue receiving, otherwise quit.
*/
using suback_handler = std::function<bool(packet_id_t packet_id,
std::vector<MQTT_NS::optional<MQTT_NS::suback_reason_code>> qoss)>;
std::vector<MQTT_NS::suback_reason_code> qoss)>;

/**
* @brief Unsubscribe handler
Expand Down Expand Up @@ -11341,19 +11341,23 @@ class endpoint : public std::enable_shared_from_this<endpoint<Mutex, LockGuard,
switch (version_) {
case protocol_version::v3_1_1:
if (h_suback_) {
std::vector<optional<suback_reason_code>> results;
// TODO: We can avoid an allocation by casting the raw bytes of the
// mqtt::buffer that is being parsed, and instead call the suback
// handler with an std::span and the mqtt::buffer (as lifekeeper)
std::vector<suback_reason_code> results;
results.resize(body.size());
std::transform(
body.begin(),
body.end(),
results.begin(),
[&](auto const& e) -> optional<suback_reason_code> {
if (e & variable_length_continue_flag) {
return nullopt;
}
else {
return static_cast<suback_reason_code>(e);
}
// http://docs.oasis-open.org/mqtt/mqtt/v3.1.1/errata01/os/mqtt-v3.1.1-errata01-os-complete.html#_Toc442180880
// The SUBACK Packet sent by the Server to the Client MUST
// contain a return code for each Topic Filter/QoS pair.
// This return code MUST either show the maximum QoS that
// was granted for that Subscription or indicate that the
// subscription failed [MQTT-3.8.4-5].
[&](auto const& e) -> suback_reason_code {
return static_cast<suback_reason_code>(e);
}
);
if (!h_suback_(info.packet_id, force_move(results))) {
Expand All @@ -11364,12 +11368,22 @@ class endpoint : public std::enable_shared_from_this<endpoint<Mutex, LockGuard,
break;
case protocol_version::v5:
if (h_v5_suback_) {
// TODO: We can avoid an allocation by casting the raw bytes of the
// mqtt::buffer that is being parsed, and instead call the suback
// handler with an std::span and the mqtt::buffer (as lifekeeper)
std::vector<v5::suback_reason_code> reasons;
reasons.resize(body.size());
std::transform(
body.begin(),
body.end(),
reasons.begin(),
// https://docs.oasis-open.org/mqtt/mqtt/v5.0/os/mqtt-v5.0-os.html#_Toc3901170
// The SUBACK packet sent by the Server to the Client MUST
// contain a Reason Code for each Topic Filter/Subscription
// Option pair [MQTT-3.8.4-6].
// This Reason Code MUST either show the maximum QoS that
// was granted for that Subscription or indicate that the
// subscription failed [MQTT-3.8.4-7].
[&](auto const& e) {
return static_cast<v5::suback_reason_code>(e);
}
Expand Down
24 changes: 12 additions & 12 deletions test/as_buffer_async_pubsub_1.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -71,11 +71,11 @@ BOOST_AUTO_TEST_CASE( pub_qos0_sub_qos0 ) {
});
c->set_suback_handler(
[&chk, &c, &pid_sub]
(packet_id_t packet_id, std::vector<MQTT_NS::optional<MQTT_NS::suback_reason_code>> results) {
(packet_id_t packet_id, std::vector<MQTT_NS::suback_reason_code> results) {
MQTT_CHK("h_suback");
BOOST_TEST(packet_id == pid_sub);
BOOST_TEST(results.size() == 1U);
BOOST_TEST(*results[0] == MQTT_NS::suback_reason_code::granted_qos_0);
BOOST_TEST(results[0] == MQTT_NS::suback_reason_code::granted_qos_0);
c->async_publish(
as::buffer("topic1", sizeof("topic1") - 1),
as::buffer("topic1_contents", sizeof("topic1_contents") - 1),
Expand Down Expand Up @@ -291,11 +291,11 @@ BOOST_AUTO_TEST_CASE( pub_qos1_sub_qos0 ) {
});
c->set_suback_handler(
[&chk, &c, &pid_pub, &pid_sub]
(packet_id_t packet_id, std::vector<MQTT_NS::optional<MQTT_NS::suback_reason_code>> results) {
(packet_id_t packet_id, std::vector<MQTT_NS::suback_reason_code> results) {
MQTT_CHK("h_suback");
BOOST_TEST(packet_id == pid_sub);
BOOST_TEST(results.size() == 1U);
BOOST_TEST(*results[0] == MQTT_NS::suback_reason_code::granted_qos_0);
BOOST_TEST(results[0] == MQTT_NS::suback_reason_code::granted_qos_0);
auto topic1 = std::make_shared<std::string>("topic1");
auto contents = std::make_shared<std::string>("topic1_contents");
pid_pub = c->async_publish(
Expand Down Expand Up @@ -511,11 +511,11 @@ BOOST_AUTO_TEST_CASE( pub_qos2_sub_qos0 ) {
});
c->set_suback_handler(
[&chk, &c, &pid_pub]
(packet_id_t packet_id, std::vector<MQTT_NS::optional<MQTT_NS::suback_reason_code>> results) {
(packet_id_t packet_id, std::vector<MQTT_NS::suback_reason_code> results) {
MQTT_CHK("h_suback");
BOOST_TEST(packet_id == 1);
BOOST_TEST(results.size() == 1U);
BOOST_TEST(*results[0] == MQTT_NS::suback_reason_code::granted_qos_0);
BOOST_TEST(results[0] == MQTT_NS::suback_reason_code::granted_qos_0);
auto topic1 = std::make_shared<std::string>("topic1");
auto contents = std::make_shared<std::string>("topic1_contents");
pid_pub = c->async_publish(
Expand Down Expand Up @@ -721,11 +721,11 @@ BOOST_AUTO_TEST_CASE( pub_qos0_sub_qos1 ) {
});
c->set_suback_handler(
[&chk, &c, &pid_sub]
(packet_id_t packet_id, std::vector<MQTT_NS::optional<MQTT_NS::suback_reason_code>> results) {
(packet_id_t packet_id, std::vector<MQTT_NS::suback_reason_code> results) {
MQTT_CHK("h_suback");
BOOST_TEST(packet_id == pid_sub);
BOOST_TEST(results.size() == 1U);
BOOST_TEST(*results[0] == MQTT_NS::suback_reason_code::granted_qos_1);
BOOST_TEST(results[0] == MQTT_NS::suback_reason_code::granted_qos_1);
c->async_publish(
as::buffer("topic1", sizeof("topic1") - 1),
as::buffer("topic1_contents", sizeof("topic1_contents") - 1),
Expand Down Expand Up @@ -949,11 +949,11 @@ BOOST_AUTO_TEST_CASE( pub_qos1_sub_qos1 ) {
});
c->set_suback_handler(
[&chk, &c, &pid_sub, &pid_pub]
(packet_id_t packet_id, std::vector<MQTT_NS::optional<MQTT_NS::suback_reason_code>> results) {
(packet_id_t packet_id, std::vector<MQTT_NS::suback_reason_code> results) {
MQTT_CHK("h_suback");
BOOST_TEST(packet_id == pid_sub);
BOOST_TEST(results.size() == 1U);
BOOST_TEST(*results[0] == MQTT_NS::suback_reason_code::granted_qos_1);
BOOST_TEST(results[0] == MQTT_NS::suback_reason_code::granted_qos_1);
auto topic1 = std::make_shared<std::string>("topic1");
auto contents = std::make_shared<std::string>("topic1_contents");
pid_pub = c->async_publish(
Expand Down Expand Up @@ -1169,11 +1169,11 @@ BOOST_AUTO_TEST_CASE( pub_qos2_sub_qos1 ) {
});
c->set_suback_handler(
[&chk, &c, &pid_sub, &pid_pub]
(packet_id_t packet_id, std::vector<MQTT_NS::optional<MQTT_NS::suback_reason_code>> results) {
(packet_id_t packet_id, std::vector<MQTT_NS::suback_reason_code> results) {
MQTT_CHK("h_suback");
BOOST_TEST(packet_id == pid_sub);
BOOST_TEST(results.size() == 1U);
BOOST_TEST(*results[0] == MQTT_NS::suback_reason_code::granted_qos_1);
BOOST_TEST(results[0] == MQTT_NS::suback_reason_code::granted_qos_1);
auto topic1 = std::make_shared<std::string>("topic1");
auto contents = std::make_shared<std::string>("topic1_contents");
pid_pub = c->async_publish(
Expand Down
Loading