Skip to content

Add better MQTT functionality #6

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
Apr 8, 2021
Merged

Add better MQTT functionality #6

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 27 additions & 8 deletions adafruit_funhouse/network.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ def __init__(
)
self._mqtt_client = None
self.mqtt_connect = None
self.mqtt_publish = None
self._mqtt_publish = None

def init_io_mqtt(self):
"""Initialize MQTT for Adafruit IO"""
Expand Down Expand Up @@ -101,7 +101,7 @@ def init_mqtt(
if use_io:
self._mqtt_client = IO_MQTT(self._mqtt_client)
self.mqtt_connect = self._mqtt_client.connect
self.mqtt_publish = self._mqtt_client.publish
self._mqtt_publish = self._mqtt_client.publish

return self._mqtt_client

Expand All @@ -120,6 +120,14 @@ def mqtt_loop(self):
except MQTT.MMQTTException as err:
print("MMQTTException: {0}".format(err))

def mqtt_publish(self, *args, **kwargs):
Copy link
Member

@brentru brentru Apr 8, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I do not understand this method's purpose - is it exposing the MQTT client's publish method and providing error handling? Why not directly call the MQTT client's publish() through this method, passing in its expected arguments instead of kwargs?

Copy link
Collaborator Author

@makermelissa makermelissa Apr 8, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's correct, but I did it this way because this works with both mini mqtt and io mqtt and the publish methods have different arguments. See:
https://github.com/adafruit/Adafruit_CircuitPython_MiniMQTT/blob/master/adafruit_minimqtt/adafruit_minimqtt.py#L556
and
https://github.com/adafruit/Adafruit_CircuitPython_AdafruitIO/blob/master/adafruit_io/adafruit_io.py#L372

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, I'll leave it for now...I disagree with using a "global" function for publishing via two separate libraries, users may get confused between the syntax of providing a feed key and an MQTT topic (differently formatted strings).

Copy link
Member

@brentru brentru Apr 8, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ideally, we'd have io_publish and mqtt_publish to really split it out. Each method would mirror the publish method to its corresponding library.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fair enough. I was trying to keep the code from growing too big, but may eventually end up going that route.

"""Publish to MQTT"""
try:
if self._mqtt_client is not None:
self._mqtt_publish(*args, **kwargs)
except OSError as err:
print("OSError: {0}".format(err))

@property
def on_mqtt_connect(self):
"""
Expand Down Expand Up @@ -147,8 +155,7 @@ def on_mqtt_disconnect(self):

@on_mqtt_disconnect.setter
def on_mqtt_disconnect(self, value):
self._get_mqtt_client()
self._mqtt_client.on_disconnect = value
self._get_mqtt_client().on_disconnect = value

@property
def on_mqtt_subscribe(self):
Expand All @@ -162,8 +169,21 @@ def on_mqtt_subscribe(self):

@on_mqtt_subscribe.setter
def on_mqtt_subscribe(self, value):
self._get_mqtt_client()
self._mqtt_client.on_subscribe = value
self._get_mqtt_client().on_subscribe = value

@property
def on_mqtt_unsubscribe(self):
"""
Get or Set the MQTT Unsubscribe Handler

"""
if self._mqtt_client:
return self._mqtt_client.on_unsubscribe
return None

@on_mqtt_unsubscribe.setter
def on_mqtt_unsubscribe(self, value):
self._get_mqtt_client().on_unsubscribe = value

@property
def on_mqtt_message(self):
Expand All @@ -177,8 +197,7 @@ def on_mqtt_message(self):

@on_mqtt_message.setter
def on_mqtt_message(self, value):
self._get_mqtt_client()
self._mqtt_client.on_message = value
self._get_mqtt_client().on_message = value

@property
def enabled(self):
Expand Down