Skip to content

refactor will_set function to match the publish function and allow the msg/payload to be encoded bytes, not just str, int or float. #221

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

Merged
merged 5 commits into from
Aug 25, 2024
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
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
8 changes: 4 additions & 4 deletions .pre-commit-config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -4,21 +4,21 @@

repos:
- repo: https://github.com/python/black
rev: 24.2.0
rev: 24.4.2
hooks:
- id: black
- repo: https://github.com/fsfe/reuse-tool
rev: v1.1.2
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think we want to change all of these versions in the pre-commit config file.

Ideally we try to keep these in sync across all of the libraries to make maintenance easier. Then we can use a patch to update them all at once.

However, I do know there are issues using the pinned version of pylint in python 3.12 and updating it to 3.x does allow it to work in that environment.

We're also in the process of switching the libraries over to use Ruff instead of pylint and Black.

I think it's best to revert these changes to the pre-commit file for now and the ruff change-over for this can be submitted as a separate PR since it's unrelated to main functional change in this PR.

Copy link
Author

@ehagerty ehagerty Aug 20, 2024

Choose a reason for hiding this comment

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

Hi @FoamyGuy , thank your, and my apologies for being thick but I don't think I intentionally changed this file. Are you asking me to make a new pull request or am I just not understanding you? Sorry for my lack of clarity :-)

Copy link
Contributor

Choose a reason for hiding this comment

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

@ehagerty no worries. It seems that those were changed in your branch accidentally. A new PR is not needed, just changing them back to the original values that they were for now.

I've added a commit to this branch just now to change them back so it is taken care of.

I've been reading up in the MQTT spec and I am in agreement with your conclusion that the last message can be a normal message just like any other. I'm going to give this a quick test today to ensure no other issues with it.

rev: v4.0.3
hooks:
- id: reuse
- repo: https://github.com/pre-commit/pre-commit-hooks
rev: v4.4.0
rev: v4.6.0
hooks:
- id: check-yaml
- id: end-of-file-fixer
- id: trailing-whitespace
- repo: https://github.com/pycqa/pylint
rev: v2.17.4
rev: v3.2.6
hooks:
- id: pylint
name: pylint (library code)
Expand Down
62 changes: 49 additions & 13 deletions adafruit_minimqtt/adafruit_minimqtt.py
Original file line number Diff line number Diff line change
Expand Up @@ -267,40 +267,76 @@ def mqtt_msg(self, msg_size: int) -> None:
if msg_size < MQTT_MSG_MAX_SZ:
self._msg_size_lim = msg_size

# pylint: disable=too-many-branches, too-many-statements
def will_set(
self,
topic: Optional[str] = None,
payload: Optional[Union[int, float, str]] = None,
qos: int = 0,
topic: str,
msg: Union[str, int, float, bytes],
retain: bool = False,
qos: int = 0,
) -> None:
"""Sets the last will and testament properties. MUST be called before `connect()`.

:param str topic: MQTT Broker topic.
:param int|float|str payload: Last will disconnection payload.
payloads of type int & float are converted to a string.
:param str|int|float|bytes msg: Last will disconnection msg.
msgs of type int & float are converted to a string.
msgs of type byetes are left unchanged, as it is in the publish function.
:param int qos: Quality of Service level, defaults to
zero. Conventional options are ``0`` (send at most once), ``1``
(send at least once), or ``2`` (send exactly once).

.. note:: Only options ``1`` or ``0`` are QoS levels supported by this library.
:param bool retain: Specifies if the payload is to be retained when
:param bool retain: Specifies if the msg is to be retained when
it is published.
"""
self.logger.debug("Setting last will properties")
self._valid_qos(qos)
if self._is_connected:
raise MMQTTException("Last Will should only be called before connect().")
if payload is None:
payload = ""
if isinstance(payload, (int, float, str)):
payload = str(payload).encode()

# check topic/msg/qos kwargs
self._valid_topic(topic)
if "+" in topic or "#" in topic:
raise MMQTTException("Publish topic can not contain wildcards.")

if msg is None:
raise MMQTTException("Message can not be None.")
if isinstance(msg, (int, float)):
msg = str(msg).encode("ascii")
elif isinstance(msg, str):
msg = str(msg).encode("utf-8")
elif isinstance(msg, bytes):
pass
else:
raise MMQTTException("Invalid message data type.")
if len(msg) > MQTT_MSG_MAX_SZ:
raise MMQTTException(f"Message size larger than {MQTT_MSG_MAX_SZ} bytes.")

self._valid_qos(qos)
assert (
0 <= qos <= 1
), "Quality of Service Level 2 is unsupported by this library."

# fixed header. [3.3.1.2], [3.3.1.3]
pub_hdr_fixed = bytearray([MQTT_PUBLISH | retain | qos << 1])

# variable header = 2-byte Topic length (big endian)
pub_hdr_var = bytearray(struct.pack(">H", len(topic.encode("utf-8"))))
pub_hdr_var.extend(topic.encode("utf-8")) # Topic name

remaining_length = 2 + len(msg) + len(topic.encode("utf-8"))
if qos > 0:
# packet identifier where QoS level is 1 or 2. [3.3.2.2]
remaining_length += 2
self._pid = self._pid + 1 if self._pid < 0xFFFF else 1
pub_hdr_var.append(self._pid >> 8)
pub_hdr_var.append(self._pid & 0xFF)

self._encode_remaining_length(pub_hdr_fixed, remaining_length)

self._lw_qos = qos
self._lw_topic = topic
self._lw_msg = payload
self._lw_msg = msg
self._lw_retain = retain
self.logger.debug("Last will properties successfully set")

def add_topic_callback(self, mqtt_topic: str, callback_method) -> None:
"""Registers a callback_method for a specific MQTT topic.
Expand Down
6 changes: 2 additions & 4 deletions adafruit_minimqtt/matcher.py
Original file line number Diff line number Diff line change
Expand Up @@ -88,11 +88,9 @@ def rec(node: MQTTMatcher.Node, i: int = 0):
else:
part = lst[i]
if part in node.children:
for content in rec(node.children[part], i + 1):
yield content
yield from rec(node.children[part], i + 1)
if "+" in node.children and (normal or i > 0):
for content in rec(node.children["+"], i + 1):
yield content
yield from rec(node.children["+"], i + 1)
if "#" in node.children and (normal or i > 0):
content = node.children["#"].content
if content is not None:
Expand Down
Loading