-
Notifications
You must be signed in to change notification settings - Fork 8
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
Added one_shot method and tests GitHub #21 #22
Conversation
@@ -450,6 +451,41 @@ sub _drop_connection { | |||
$self->{last_connect} = 0; | |||
} | |||
|
|||
sub one_shot { | |||
#Copyright: Michael R. Davis (c) 2023 | |||
#License: MIT |
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.
Any contributions will have to be licensed under the same terms as the module itself. I will not have individual copyright notices or licenses for individual pieces of code.
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.
Pick your favourite OSI approved license :)
Your license is the same as MIT. It's just not explicit.
my $self = shift; | ||
my $topic_sub = shift or die("Error: subscribe topic is required"); | ||
my $topic_pub = shift or die("Error: publish topic is required"); | ||
my $message = shift; $message = '' unless defined $message; #default '', allow 0 and support perl 5.8 |
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.
The formatting of the code is very different from the rest of the module. For additions to be taken seriously, please consider trying to follow the style of the existing code. This also applies to the rest of the suggested changes.
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.
Do you have a perltidy
configuration?
last if $found; | ||
} | ||
|
||
$self->unsubscribe($topic_sub); #must unsubscribe to do one_shot back to back |
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.
Why?
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.
Why?
last if $found;
This is required to get the "first" found message from the broker that matches the topic.
$self->unsubscribe($topic_sub); #must unsubscribe to do one_shot back to back
I'm not an MQTT expert but, in my testing, the unsubscribe
was required to send a second one_shot
to the device or it would not capture the return message. I assume that the previous subscription was still calling the old anonymous sub routine and then updating the old my
variables and thus not getting to the second subscription that was added.
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.
Thanks for taking the time to contribute an addition to Net::MQTT::Simple.
Since you requested a review, I've reviewed your changes. But please note that I have no plans for adding or merging this functionality to Net::MQTT::Simple at this point, even if the concerns would be addressed, because I'm not convinced this functionality should be in the module, per my comment in #21.
Please review my implementation of this one_shot method to resolve my issue #21.