Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

MQTTV5Client API #244

Merged
merged 6 commits into from
Aug 8, 2023
Merged

MQTTV5Client API #244

merged 6 commits into from
Aug 8, 2023

Conversation

CIPop
Copy link

@CIPop CIPop commented Aug 3, 2023

This PR contains a proposal for a new, blocking MQTTV5Client for C.
Important: While the build system will not fail, none of the new functionality is actually implemented. The implementation and testing will be part of a series of future PRs.

The PR contains commits from develop not yet in the mqttv5 branch (only the latest 3 commits are new). These should automatically disappear after a merge of develop into mqttv5.

Design

MQTTClient is reused as much as possible

  • #if defined(MQTTV5) used throughout MQTTClient.h/c
  • New MQTTClient functionality (for parity with V5):
    • MQTTPublishWithResults that exposes PUBACK/PUBCOMP information through a new PubDoneData struct.

MQTTV5 Client differences:

  • Init will require a reusable receive MQTTProperties* struct, allocated by the application. Behavior regarding running out of property space can also be controlled using the truncateRecvProperties parameter.
  • All V5 APIs accept or return MQTTProperties
  • New APIs, only available in V5:
    - MQTTV5Auth and MQTTV5SetAuthHandler (controlHandler)
    - MQTTV5SetDisconnectHandler(controlHandler)
    - MQTTV5UnsubscribeWithResults + MQTTV5UnsubackData

Documentation

All documentation for MQTTClient-C and MQTTV5Client-C has been updated.

/// @brief The MQTTv5 message properties.
MQTTProperties* properties;
/// @brief The MQTTv5 reason code.
enum MQTTReasonCodes reasonCode;
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@icraggs and others, I'm not certain about replacing the existing unsigned char rc with the enum: while it makes it easier to discover and understand the returned codes, I believe sizeof(enum) > sizeof(char).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@CIPop there should certainly be two variables, I think. ReasonCode for MQTT5. While sizeof(enum) > sizeof(char) we know that in this case the number of enums is not going to exceed the capacity of the one byte, so the conversion should work fine? If it's preferred to not use an enum because of the conversion, we could just use a list of constants instead (while retaining the enum info as comments perhaps).

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds good: reasonCode as an enum makes the available values a lot more discoverable.

I wasn't aware of this but some compilers can optimize the size (even if it is non-C standard): e.g. -fshort-enums

@icraggs icraggs changed the base branch from mqttv5 to develop August 8, 2023 11:41
@icraggs icraggs changed the base branch from develop to mqttv5 August 8, 2023 11:41
@icraggs icraggs merged commit 5d7a67c into eclipse-paho:mqttv5 Aug 8, 2023
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants