-
Notifications
You must be signed in to change notification settings - Fork 51
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
Changes from 4 commits
Commits
Show all changes
5 commits
Select commit
Hold shift + click to select a range
2197ad7
refactor will_set function to match the publish function to allow the…
88787e6
trying to correctly set up pre-commit
65ea984
trying to correctly set up pre-commit
2d3708d
having to fix adafruit code to migrate to yield-from
9c0ecfc
revert pre-commit versions
FoamyGuy File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
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.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
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 :-)
There was a problem hiding this comment.
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.