MQTTv5. on_publish. Added reasonCode, properties arguments
💯 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:
- if doable reliably detect the number of argument accepted by on_publish and adapt argument send accordingly.
- If the detection isn't doable or reliable enough (I think to
*argsor**kwargscases), we might make it an option. But I think the option should not be tied to protocol used.
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.
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.