GoogleCloudPlatform / iot-device-sdk-embedded-c

Cloud IoT Device SDK for Connectivity to IoT Core.
Other
247 stars 83 forks source link

Unable to receive commands with a subfolder #84

Closed rohansingh closed 4 years ago

rohansingh commented 5 years ago

When sending commands to a device, they may be sent to the bare /commands topic, or any sub-topic thereof (eg., /commands/X).

The device subscribes to the /commands/# wildcard topic, so it should receive messages for any sub-topics as well.

However, if a message is received on a sub-topic, it is erroneously discarded. For example:

[-1833130529][iotc_mqtt_logic_layer.c:272 (iotc_mqtt_logic_layer_pull)] [m.id[0] m.type[3]] received msg
[-1833130520][iotc_mqtt_logic_layer_publish_handler.h:48 (call_topic_handler)] [m.id[0]] looking for publish message handler
[-1833130509][iotc_mqtt_logic_layer_publish_handler.h:70 (call_topic_handler)] [m.id[0]] received publish message for topic which is not registered
message
  type:              3
  qos:               0
  dup:               false
  retain:            false
  message_id:        0
topic_name:
[54] /devices/honestly-skilled-guiding-hog/commands/screens

There is no reasonable workaround for this issues. Subscribing to specific sub-topics in /commands is not allowed, devices are required to subscribe to the wildcard topic.

rohansingh commented 5 years ago

I believe this can be fixed in cmp_topics.

rohansingh commented 5 years ago

Here's a really rough patch for this:

diff --git a/src/libiotc/iotc_user_sub_call_wrapper.c b/src/libiotc/iotc_user_sub_call_wrapper.c
index 4bd5126..c63e004 100644
--- a/src/libiotc/iotc_user_sub_call_wrapper.c
+++ b/src/libiotc/iotc_user_sub_call_wrapper.c
@@ -73,7 +73,10 @@ iotc_state_t iotc_user_sub_call_wrapper(void* context, void* data,
           msg->publish.content ? msg->publish.content->data_ptr : NULL;
       params.message.temporary_payload_data_length =
           msg->publish.content ? msg->publish.content->length : 0;
-      params.message.topic = (const char*)sub_data->subscribe.topic;
+      params.message.topic =
+          msg->publish.topic_name
+              ? (const char*)msg->publish.topic_name->data_ptr
+              : NULL;

       in_state = iotc_mqtt_convert_to_qos(msg->common.common_u.common_bits.qos,
                                           &params.message.qos);
diff --git a/src/libiotc/mqtt/logic/iotc_mqtt_logic_layer_data_helpers.h b/src/libiotc/mqtt/logic/iotc_mqtt_logic_layer_data_helpers.h
index 77d229d..874b95d 100644
--- a/src/libiotc/mqtt/logic/iotc_mqtt_logic_layer_data_helpers.h
+++ b/src/libiotc/mqtt/logic/iotc_mqtt_logic_layer_data_helpers.h
@@ -38,6 +38,14 @@ static inline int8_t cmp_topics(const union iotc_vector_selector_u* a,
     return 0;
   }

+  const size_t aLen = strlen(ca->subscribe.topic);
+  if ((cb->length > aLen) && (ca->subscribe.topic[aLen - 2] == '/') &&
+      (ca->subscribe.topic[aLen - 1] == '#')) {
+    if (memcmp(ca->subscribe.topic, cb->data_ptr, aLen - 1) == 0) {
+      return 0;
+    }
+  }
+
   return 1;
 }

I'm not very familiar with this code (or C in general), so I'm not sure if this is a decent approach. If it seems reasonable, I'm happy to create a PR with some unit tests.

atigyi commented 5 years ago

Thank you for spotting the lack of /commands/# support and for offering a solution! Indeed, this looks like a reasonable solution! Please feel free to create a PR with this idea! Please target the development branch with the PR.

Few minor nitpicking notes though:

Thanks in advance!

atigyi commented 4 years ago

Closing this issue, #89 solves it. Thanks for @sheindel!