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

Change messageReceived signature? #38

Closed
256dpi opened this issue May 17, 2016 · 8 comments
Closed

Change messageReceived signature? #38

256dpi opened this issue May 17, 2016 · 8 comments

Comments

@256dpi
Copy link
Owner

256dpi commented May 17, 2016

I have the feeling the current signature is a little complicated for amateurs.

So rather than:

void messageReceived(String topic, String payload, char * bytes, unsigned int length) {
  Serial.print("incoming: ");
  Serial.print(topic);
  Serial.print(" - ");
  Serial.print(payload);
  Serial.println();
}

We could have something like this:

void messageReceived(MQTTMessage msg) {
  Serial.print("incoming: ");
  Serial.print(msg.topic());
  Serial.print(" - ");
  Serial.print(msg.payload());
  Serial.println();
}

Then we would also have the ability to add more convenience functions like:

if (msg.topicBeginsWith("/namespace")) {
  // do something
}
@256dpi 256dpi added this to the v2 milestone May 17, 2016
@256dpi
Copy link
Owner Author

256dpi commented May 17, 2016

Actually this would also fix a serious bug as creating a String from the payload using String(payload) allocates another buffer with the exact copy.

@gibix
Copy link
Contributor

gibix commented May 18, 2016

Is not very user friendly, but can be better!
You can add the sign if MQTTMessage in the readme or as comment in the examples?

@256dpi
Copy link
Owner Author

256dpi commented May 24, 2016

What is not very user friendly? The current implementation or this proposed change?
Sorry, I read your answer now various times but still don't get it... 😄

This would be a breaking change and result in releasing a v2.

@gibix
Copy link
Contributor

gibix commented May 24, 2016

Sorry for the language ;)
I'm saying that a lot's of arduino users may not be friendly with data structures and usually in the libraries are avoided, but if you don't want to hack the MQTTClient.h code I agree that use the MQTTMessage in the callback is the best way.
I suggest that in the readme we should add the structure description, like:

typedef struct {
    char * topic;
    char * payload;
    unsigned int length;
    boolean retained;
} MQTTMessage;

or in the examples as a comment.

@256dpi
Copy link
Owner Author

256dpi commented May 25, 2016

Thanks for the clarifications.
I'm not yet fully convinced by my own idea, so I have to sleep on it again some time. ;)

I'll keep you updated.

@256dpi
Copy link
Owner Author

256dpi commented Jul 8, 2016

Note to myself: To support multiple clients at the same time the signature could even be updated to something like this:

void messageReceived(MQTTMessage msg, MQTTClient* client) {
  Serial.print("incoming: ");
  Serial.print(msg.topic());
  Serial.print(" - ");
  Serial.print(msg.payload());
  Serial.println();
}

@sandeepmistry
Copy link
Contributor

Regarding #38 (comment), in general it would be nice to avoid pointers in the call back, as from my understanding most Arduino users aren't exposed to them.

@256dpi
Copy link
Owner Author

256dpi commented May 3, 2017

Has been simplified in #59.

@256dpi 256dpi closed this as completed May 3, 2017
@256dpi 256dpi mentioned this issue May 3, 2017
Merged
11 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants