Bridge notifications behavior for WITH_ADNS=y and WITH_ADNS=n option

[marcinkowskip]

Hello,

According to the topic. Is there any reason why notifications don't work the same way based on the option? I am talking about lines:
https://github.com/eclipse/mosquitto/blob/master/src/bridge.c#L180
https://github.com/eclipse/mosquitto/blob/master/src/bridge.c#L358

The second function has if statement.

I run into this problem when I wanted to enable local notifications for AWS bridge, but I could not because lack of the condition. I locally changed line 180 to match 358 (copied if statement) and everything worked correctly.

Thanks,
PM

[karlp]

related #1488

[marcinkowskip]

Ok, I spent more time on this and problem is more complicated. It seems that notifications and notifications_local_only uses the same mechanism (LWT) to send status. If we don't set LWT, we would only get information about first connection (since status is retained, only updated when we create bridge connection, and disconnecting uses LWT). If we set LWT this LWT will be sent to remote broker even if we have notifications_local_only set to true (https://github.com/eclipse/mosquitto/blob/master/src/bridge.c#L180).

The easiest quick fix for that would be to use LWT, but not send it if notifications_local_only is enabled. I am not sure if it is proper fix since I spent 2-3h to analyze and test my fix (attached). This also fixes #1488.

diff --git a/lib/send_connect.c b/lib/send_connect.c
index 2ad8fdf..c2137bd 100644
--- a/lib/send_connect.c
+++ b/lib/send_connect.c
@@ -100,7 +100,11 @@ int send__connect(struct mosquitto *mosq, uint16_t keepalive, bool clean_session
 	}else{
 		payloadlen = 2;
 	}
+#ifdef WITH_BROKER
+	if(mosq->will && (!mosq->bridge || !mosq->bridge->notifications_local_only)){
+#else
 	if(mosq->will){
+#endif
 		will = 1;
 		assert(mosq->will->msg.topic);
 
diff --git a/src/bridge.c b/src/bridge.c
index c951c8c..e3cd9dd 100644
--- a/src/bridge.c
+++ b/src/bridge.c
@@ -355,12 +355,10 @@ int bridge__connect(struct mosquitto_db *db, struct mosquitto *context)
 				context->bridge->initial_notification_done = true;
 			}
 
-			if (!context->bridge->notifications_local_only) {
-				notification_payload = '0';
-				rc = will__set(context, context->bridge->notification_topic, 1, &notification_payload, 1, true, NULL);
-				if(rc != MOSQ_ERR_SUCCESS){
-					return rc;
-				}
+			notification_payload = '0';
+			rc = will__set(context, context->bridge->notification_topic, 1, &notification_payload, 1, true, NULL);
+			if(rc != MOSQ_ERR_SUCCESS){
+				return rc;
 			}
 		}else{
 			notification_topic_len = strlen(context->bridge->remote_clientid)+strlen("$SYS/broker/connection//state");
@@ -375,13 +373,11 @@ int bridge__connect(struct mosquitto_db *db, struct mosquitto *context)
 				context->bridge->initial_notification_done = true;
 			}
 
-			if (!context->bridge->notifications_local_only) {
-				notification_payload = '0';
-				rc = will__set(context, notification_topic, 1, &notification_payload, 1, true, NULL);
-				mosquitto__free(notification_topic);
-				if(rc != MOSQ_ERR_SUCCESS){
-					return rc;
-				}
+			notification_payload = '0';
+			rc = will__set(context, notification_topic, 1, &notification_payload, 1, true, NULL);
+			mosquitto__free(notification_topic);
+			if(rc != MOSQ_ERR_SUCCESS){
+				return rc;
 			}
 		}
 	}

[marcinkowskip]

With this fix, you can bridge to AWS (which does not support LWT) and have local bridge status (notifications true, notifications_local_only true).

[joonaspessi]

@marcinkowskip we have been struggling with the same issue connecting Mosquitto bridge to AWS IoT with configuration (notifications true and notifications_local_only true) without success. This is really nice that you found out the issue with LWT. I think this is probably a valid issue for all brokers that don't support retained messages?

[marcinkowskip]

I think this is probably a valid issue for all brokers that don't support retained messages?

Not sure if I follow you. The issue is not related to retained messages at all. The issue is strictly related to sharing LWT message to notify bridge status for both remote and local setup. If you set notifications_local_only true the LWT will not be created, and since LWT is missing you wont get message on disconnect event. If you will have notifications_local_only false the LWT will be created, but AWS will disconnect your connection on CONNECT message, because it is does not support LWT mechanism. The patch I sent fixes both cases because it will create LWT message for both notifications_local_only true and notifications_local_only false, but the LWT will not be sent if you set notifications_local_only true.

[ralight]

This looks good to me, thanks very much!