eclipse-paho / paho.mqtt.embedded-c

Paho MQTT C client library for embedded systems. Paho is an Eclipse IoT project (https://iot.eclipse.org/)
https://eclipse.org/paho
Other
1.37k stars 757 forks source link

Public API: avoid name collisions #243

Closed CIPop closed 1 year ago

CIPop commented 1 year ago

Repurposing to include fix for #180:

This is a public API breaking change which will result in a major version release.

CIPop commented 1 year ago

Error when trying to build a new MQTTV5Client:

FAILED: MQTTClient-C/src/CMakeFiles/paho-embed-mqtt5cc.dir/MQTTClient.c.o 
/usr/bin/cc -DMQTTCLIENT_PLATFORM_HEADER=MQTTLinux.h -DMQTTCLIENT_QOS2=1 -DMQTTV5 -Dpaho_embed_mqtt5cc_EXPORTS -I/home/crispop/paho.mqtt.embedded-c/MQTTPacket/src -I/home/crispop/paho.mqtt.embedded-c/MQTTClient-C/src/linux -g -fPIC -MD -MT MQTTClient-C/src/CMakeFiles/paho-embed-mqtt5cc.dir/MQTTClient.c.o -MF MQTTClient-C/src/CMakeFiles/paho-embed-mqtt5cc.dir/MQTTClient.c.o.d -o MQTTClient-C/src/CMakeFiles/paho-embed-mqtt5cc.dir/MQTTClient.c.o -c /home/crispop/paho.mqtt.embedded-c/MQTTClient-C/src/MQTTClient.c
In file included from /home/crispop/paho.mqtt.embedded-c/MQTTClient-C/src/V5/MQTTV5Client.h:22,
                 from /home/crispop/paho.mqtt.embedded-c/MQTTClient-C/src/MQTTClient.c:19:
/home/crispop/paho.mqtt.embedded-c/MQTTClient-C/src/V5/../MQTTClient.h:61:55: error: redeclaration of enumerator ‘SUCCESS’
   61 | enum returnCode { BUFFER_OVERFLOW = -2, FAILURE = -1, SUCCESS = 0 };
      |                                                       ^~~~~~~
In file included from /home/crispop/paho.mqtt.embedded-c/MQTTPacket/src/V5/MQTTV5Packet.h:26,
                 from /home/crispop/paho.mqtt.embedded-c/MQTTClient-C/src/V5/../MQTTClient.h:38,
                 from /home/crispop/paho.mqtt.embedded-c/MQTTClient-C/src/V5/MQTTV5Client.h:22,
                 from /home/crispop/paho.mqtt.embedded-c/MQTTClient-C/src/MQTTClient.c:19:
/home/crispop/paho.mqtt.embedded-c/MQTTPacket/src/V5/MQTTReasonCodes.h:18:3: note: previous definition of ‘SUCCESS’ was here
   18 |   SUCCESS = 0,
      |   ^~~~~~~
icraggs commented 1 year ago

I had put my comments in #180:

n the main C client, I prefixed all constants with either MQTTCLIENT or MQTTASYNC, and the V5 reason codes with MQTTREASONCODE_

I'm not saying those are the best choices, but we could use the same convention for the sake of making the APIs similar.

The other thought I had was to prefix them all with PAHO_ to distinguish between other APIs. But I would advocate following the main C client API convention, unless there's a specific objection, to get more of a family feel.

If we're going for a significant renaming exercise, which I think we should for these enums/constants, then we'll need a major version change to signify that (2.0.0).

icraggs commented 1 year ago

I've recreated the mqttv5 branch and updated it match develop, so we can use that as a working branch for a V5 MQTTClient layer

CIPop commented 1 year ago

If we're going for a significant renaming exercise, which I think we should for these enums/constants, then we'll need a major version change to signify that (2.0.0).

I agree as well:

  1. MQTTClient-C will follow Paho Main.
  2. It's a good idea to change all of them now.
  3. I will also propose a separate PR with changes to the file-names to differentiate between MQTTClient for C, MQTTClient for C++, etc.

I can re-use this PR to make all changes towards develop.

I've recreated the mqttv5 branch and updated it match develop, so we can use that as a working branch for a V5 MQTTClient layer

Thank you! I will start submitting partial work, starting with the MQTTV5Client-C API surface PR there.

CIPop commented 1 year ago

@icraggs PTAL - I've included many other changes (unfortunately this might have brought the line-count > 1k) A future PR should change the file-names to disambiguate between the C++ and C clients.

CIPop commented 1 year ago

@MikhailNatalenko please let me know if this addresses all your concerns from #180.

CIPop commented 1 year ago

Rebased on top of develop. ctest passed locally.