1technophile / OpenMQTTGateway

MQTT gateway for ESP8266 or ESP32 with bidirectional 433mhz/315mhz/868mhz, Infrared communications, BLE, Bluetooth, beacons detection, mi flora, mi jia, LYWSD02, LYWSD03MMC, Mi Scale, TPMS, BBQ thermometer compatibility & LoRa.
https://docs.openmqttgateway.com
GNU General Public License v3.0
3.61k stars 792 forks source link

Receiver acknowledgement ignored during send with MQTTtoRFM69 #376

Closed aussieW closed 5 years ago

aussieW commented 5 years ago

When sending a message to a receiver via MQTTtoRFM69 there appears to be 10 attempts. Each time a message is sent it tests for an acknowledgement from the receiver via if(radio.sendWithRetry(valueRCV, data, strlen(data))) {

If the above condition is true I believe that it should then break out of the loop. Instead, it ignores acknowledgement and continues to resend the message all 10 times.

A changes to the code something like the following would cause a message to be resent only until it is received and acknowledged. `while (loops--) { if(radio.sendWithRetry(valueRCV, data, strlen(data))) { deltaMillis = millis() - startMillis; trc(F(" OK ")); trc(deltaMillis);

      // Acknowledgement to the GTWRF topic
      char buff[sizeof(subjectGTWRFM69toMQTT)+4];
      sprintf(buff, "%s/%d", subjectGTWRFM69toMQTT, radio.SENDERID);
      pub(buff, data);
      break;  // <<---- Abort once the message is acknowledged.

}`

aussieW commented 5 years ago

I notice that the function sendWithRetry() takes a parameter for the number of retries, which it manages itself. Maybe this feature could be used to simplify the MQTTtoRFM69() function instead of looping 10 times.

1technophile commented 5 years ago

I corrected it, if you could test that it is working with your setup it would be great @aussieW

aussieW commented 5 years ago

Hi @1technophile,

Sorry for the delay but I have been overseas for the last two weeks.

I downloaded v0.9.1 and tested the changes. Unfortunately they do not seem to work. The problem being that even if a message is not received or sent to a non-existent node, it still acknowledges that it was sent successfully. I can't really tell what is wrong but my code (see below) which is fairly similar seems to work okay.

#ifdef simpleReceiving
  void MQTTtoRFM69(char * topicOri, char * datacallback) {

    String topic = topicOri;

    if (topic.substring(0, strlen(subjectMQTTtoRFM69)) == subjectMQTTtoRFM69) {
      trc(F("MQTTtoRFM69 data analysis"));
      char data[RF69_MAX_DATA_LEN+1];
      memcpy(data, (void *)datacallback, RF69_MAX_DATA_LEN);
      data[RF69_MAX_DATA_LEN] = '\0';

      //We look into the subject to see if a special RF protocol is defined
      int valueRCV = defaultRFM69ReceiverId; //default receiver id value
      int pos = topic.lastIndexOf(RFM69receiverKey);
      if (pos != -1){
        pos = pos + +strlen(RFM69receiverKey);
        valueRCV = (topic.substring(pos,pos + 3)).toInt();
        trc(F("RFM69 receiver ID:"));
        trc(valueRCV);
      }

      if(radio.sendWithRetry(valueRCV, data, strlen(data), 10)) {  
        //Publish an acknowledgement to the GTWRF topic
        pub(subjectGTWRFM69toMQTT, data);
        trc(F("Sent successfully"));
      }
      else {
        trc(F("MQTTtoRFM69 sending failed"));
      }
    }
  }

#endif
#ifdef jsonReceiving
  void MQTTtoRFM69(char * topicOri, JsonObject& RFM69data) {

   if (strcmp(topicOri,subjectMQTTtoRFM69) == 0){
      const char * data = RFM69data["data"];
      trc(F("MQTTtoRFM69 json data analysis"));
      if(data){
        trc(F("MQTTtoRFM69 data ok"));
        int valueRCV = RFM69data["receiverid"]| defaultRFM69ReceiverId; //default receiver id value
        trc(F("RFM69 receiver ID:"));
        trc(valueRCV);
        if(radio.sendWithRetry(valueRCV, data, strlen(data), 10)) {
          //Publish an acknowledgement to the GTWRF topic
          pub(subjectGTWRFM69toMQTT, RFM69data);// we acknowledge the sending by publishing the value to an acknowledgement topic, for the moment even if it is a signal repetition we acknowledge also
          trc(F("Sent successfully"));
        } else {
          trc(F("MQTTtoRFM69 sending failed"));
        }
      } else {
        trc(F("MQTTtoRFM69 Fail reading from json"));
      }
    }
  }
#endif
1technophile commented 5 years ago

thanks, a wrong parenthesis placement ..., now corrected into the dev branch