retain breaks old code - wippersnapper CI builds

[tyeth]

Not sure myself as not a c/c++ expert, but it's kicking off about ambiguity between the uint8/16 version of publish with qos and the version with retain+qos. I get that the bool false represents 0 due to a define, but I would have thought it would still only pick the overload with the correct parameter count and typelist (not const versions).

Seems to be related to #216

[tyeth]

https://github.com/adafruit/Adafruit_Wippersnapper_Arduino/actions/runs/3451873666/jobs/5761396456#step:8:120 Both lines in this error message are missing the QOS parameter which are on the next lines in the MQTT library.

 /home/runner/Arduino/libraries/Adafruit_Wippersnapper_Arduino/src/Wippersnapper.cpp: In function 'void publishI2CResponse(wippersnapper_signal_v1_I2CResponse*)':
  /home/runner/Arduino/libraries/Adafruit_Wippersnapper_Arduino/src/Wippersnapper.cpp:466:79: error: call of overloaded 'publish(char*&, uint8_t [512], size_t&, int)' is ambiguous
     WS._mqtt->publish(WS._topic_signal_i2c_device, WS._buffer_outgoing, msgSz, 1);
                                                                                 ^
  In file included from /home/runner/Arduino/libraries/Adafruit_Wippersnapper_Arduino/src/Wippersnapper.h:51,
                   from /home/runner/Arduino/libraries/Adafruit_Wippersnapper_Arduino/src/Wippersnapper.cpp:34:
  /home/runner/Arduino/libraries/Adafruit_MQTT_Library/Adafruit_MQTT.h:195:8: note: candidate: 'bool Adafruit_MQTT::publish(const char*, uint8_t*, uint16_t, uint8_t)'
     bool publish(const char *topic, uint8_t *payload, uint16_t bLen,
          ^~~~~~~
  /home/runner/Arduino/libraries/Adafruit_MQTT_Library/Adafruit_MQTT.h:197:8: note: candidate: 'bool Adafruit_MQTT::publish(const char*, uint8_t*, uint16_t, bool, uint8_t)'
     bool publish(const char *topic, uint8_t *payload, uint16_t bLen, bool retain,
          ^~~~~~~

[tyeth]

I had a quick play and removing the default values for qos in the header file allowed my wippersnapper build to get past the ambiguity, but that feels not great as it wasn't added to line 192 etc in #216. Played with parameter order and defaults but didn't help. Then occurred to me to remove the default values for just the new overloads, avoiding breaking the default QOS value in old code examples, but enforcing the providing of both qos+retain if specifying retain. Don't know if that's acceptable or not @ben-willmore @brentru

This was what I ended up with in .h file lines 192-198:

  bool publish(const char *topic, const char *payload, uint8_t qos = 0);
  bool publish(const char *topic, const char *payload, uint8_t qos,
               bool retain);
  bool publish(const char *topic, uint8_t *payload, uint16_t bLen,
               uint8_t qos = 0);
  bool publish(const char *topic, uint8_t *payload, uint16_t bLen, 
               uint8_t qos, bool retain);

-Edit- Whoops, forgot I left the order changed, well you get the idea, by not specifying QOS in the second overloaded forms which include retain, the ambiguity goes away.

-Edit2- When I change the parameter order back, and leave QOS default specified, then everything breaks again, I guess due to the value for QOS being misinterpreted as a value suitable for retain (bool) in the wippersnapper example.

[ben-willmore]

You're right -- it's a bit surprising that this is ambiguous but it does make sense. Unless I misunderstood your second edit, I don't see a better solution to this than moving retain to the end of the argument list and giving it a default value, as in https://github.com/ben-willmore/Adafruit_MQTT_Library/tree/retain-default-parameters .

[tyeth]

Yeah i think that makes most sense looking at what you've done in that branch, but to be honest I suck at c/c++, more of a c# guy sorry and haven't got time to retest this moment.

[brentru]

@ben-willmore your commit in https://github.com/ben-willmore/Adafruit_MQTT_Library/commit/83af73f4e52a96d40d73ff2b258605fcc0d18bfb makes sense and fixes this issue. Are you OK with checking out a PR? I can test WipperSnapper against that.