MQTTv5. on_publish. Added reasonCode, properties arguments #656
Conversation
|
💯 Please add this in! This is very necessary |
|
Thank you for this great contribution! I stumbled into the same issue. With the help of your PR, I can now check the reasoncode in the on_publish-callback. It is the only way to find out if my published message has been rejected due to access control. Next, we should also modify the return code of the publish function itself. Currently it is always Success - in contrast to the mqtt lib for nodejs, for example. (I described this in more detail in issue #715) |
|
It would be fantastic to get this merged in, we recently came across a bug related to this and have no workaround until this is merged in. |
|
This will cause a breaking change. Any user that have a working client with MQTTv5 (but don't need reasonCode or properties) will have their client broken. My two idea to avoid this is:
Also could you add a "Signed-Off" to your commit (option "-s" to git commit CLI). Eclipse ask us to verify its presence, which serve as confirmation that you agree with the ECA. |
| mid: matches the mid variable returned from the corresponding | ||
| publish() call, to allow outgoing messages to be tracked. | ||
| reasonCodes: the MQTT v5.0 reason codes received from the broker for each | ||
| subscription. A optional ReasonCodes instances. May ne None. |
There was a problem hiding this comment.
| subscription. A optional ReasonCodes instances. May ne None. | |
| subscription. A optional ReasonCodes instances. May be None. |
|
I've just seen that we already had something similar to you change with the on_connect callback. But I still think we should avoid breaking. After a second though I think a explicit option to decide which callback API to use is better than auto-detection. |
|
Thank for your contribution. Closing this PR because #802 fix the same issue as this PR. |
No description provided.