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

Pass Message object to MQTT message callbacks #21959

Merged
merged 6 commits into from Mar 13, 2019

Conversation

Projects
None yet
4 participants
@emontnemery
Copy link
Contributor

commented Mar 12, 2019

Description:

Pass Message object to MQTT message callbacks instead of topic, message and qos. These values are now available on the passed in message object: msg.topic, msg.message, msg.qos.

For backwards compatibility, wrap callbacks using the old signature and print a message when subscribing.
Severity level of the deprecation message is currently INFO, this will be raised to warning, and finally the backwards compatible wrapping will be removed.

Background:
We currently pass topic,, payload and qos to the callback, omitting retain flag.
Access to the retain flag will be highly useful in upcoming frontend including possibility to subscribe to topics and view received MQTT messages.

Checklist:

  • The code change is tested and works locally.
  • Local tests pass with tox. Your PR cannot be merged unless tests pass
  • There is no commented out code in this PR.

@emontnemery emontnemery requested a review from home-assistant/core as a code owner Mar 12, 2019

@ghost ghost assigned emontnemery Mar 12, 2019

@ghost ghost added the in progress label Mar 12, 2019


topic = attr.ib(type=str)
payload = attr.ib(type=PublishPayloadType)
qos = attr.ib(type=int, default=0)

This comment has been minimized.

Copy link
@balloob

balloob Mar 13, 2019

Member

Why do we need a default? Doesn't the message callback always provide one?

This comment has been minimized.

Copy link
@emontnemery

emontnemery Mar 13, 2019

Author Contributor

Changed.
Message class was already used to hold will and birth messages, but it should be fine to change since will and birth conf schema has default value for qos and retain.

_LOGGER.info(
"Signature of MQTT msg_callback '%s.%s' is deprecated",
inspect.getmodule(msg_callback).__name__, msg_callback.__name__)
async_remove = await hass.data[DATA_MQTT].async_subscribe(

This comment has been minimized.

Copy link
@balloob

balloob Mar 13, 2019

Member

Can we keep subscribe call outside of the if.

"Exception in {} when handling msg on '{}': '{}'".format(
msg_callback.__name__, topic, msg)),
qos, encoding)
def wrap_msg_callback(

This comment has been minimized.

Copy link
@balloob

balloob Mar 13, 2019

Member

Let's move this outside of this function

@balloob

This comment has been minimized.

Copy link
Member

commented Mar 13, 2019

oops never pressed submit on my review comments.

@balloob

This comment has been minimized.

Copy link
Member

commented Mar 13, 2019

Marking as breaking change so we can inform people with custom components

emontnemery added some commits Mar 11, 2019

@emontnemery emontnemery force-pushed the emontnemery:mqtt_callbacks_retain branch from 9e859a9 to 3e8c92a Mar 13, 2019

@emontnemery

This comment has been minimized.

Copy link
Contributor Author

commented Mar 13, 2019

@balloob Thanks for reviewing! The comments should be fixed now.

@balloob balloob merged commit 5957e4b into home-assistant:dev Mar 13, 2019

4 checks passed

Hound No violations found. Woof!
cla-bot Everyone involved has signed the CLA
continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls Coverage increased (+0.004%) to 92.831%
Details

@ghost ghost removed the in progress label Mar 13, 2019

@hdsheena

This comment has been minimized.

Copy link

commented Mar 14, 2019

I'm still a rote beginner with MQTT but does this change affect an example like this one? https://www.home-assistant.io/cookbook/python_component_mqtt_basic/

@emontnemery

This comment has been minimized.

Copy link
Contributor Author

commented Mar 14, 2019

@hdsheena yes, good point, the example should be updated
Edit: done, home-assistant/home-assistant.io#8932

@balloob balloob referenced this pull request Mar 20, 2019

Merged

0.90.0 #22216

@emontnemery emontnemery deleted the emontnemery:mqtt_callbacks_retain branch Apr 3, 2019

@emontnemery emontnemery referenced this pull request Apr 6, 2019

Merged

Raise severity of MQTT callback deprecation warning #22792

1 of 3 tasks complete
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.