Skip to content
This repository has been archived by the owner on Mar 10, 2020. It is now read-only.

Pass complete packet to Ascoltatore.publish #53

Closed
davedoesdev opened this issue May 16, 2013 · 3 comments
Closed

Pass complete packet to Ascoltatore.publish #53

davedoesdev opened this issue May 16, 2013 · 3 comments

Comments

@davedoesdev
Copy link
Collaborator

I'd like to access the qos and messageId properties of the mqtt message received.
At the moment, publish() just gets the topic and payload.
Adding (or replacing) with the packet is likely to change the API.
I'm happy to do the change but should this be an extra arg but which to choose?

Now: publish(topic, message, done)
Just pass packet: publish(packet, done)
Add packet: publish(topic, message, packet, done)
Backwards compatible: publish(topic, message, done, packet)

?

@mcollina
Copy link
Collaborator

I've been thinking into adding this feature for quite a while. I don't have much time to work on this now, but if you want to submit a PR we can discuss about it.
If It's a big refactoring, though I can provide some guidance.

Moreover, I'm not in favor of specifying the packet, but some options for constructing the packet (e.g. the qos for MQTT), like so:

publish(topic, message, options, done)

The 3-parameters API is frozen, but we can support a 4-params API with https://developer.mozilla.org/en-US/docs/JavaScript/Reference/Global_Objects/Function/length.

If you want to take charge of this issue, I think the behavior for this should be putted inside AbstractAscoltatore, in a public publish method that calls a private _publish method that the subclasses implements, which will use 4 parameters.
The only responsibility of the public publish method is to transform a 3-params calls into a 4-params call.

You should also need to pass it to the subscribe callback.

(Remember to check if your code lints, through the make jshint task)

@mcollina
Copy link
Collaborator

Just to clarify, in MQTTAscoltatore you can pass 3 parameters to the subscribed callback:

  ascoltatore.subscribe("hello/*", function(topic, message, packet) { ... });

@mcollina
Copy link
Collaborator

Thanks again for the contribution!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants