taxilian / OctoPrint-Twilio

Octoprint plugin for print completion notifications using Twilio (for free w/ trial account)
GNU Affero General Public License v3.0
5 stars 9 forks source link

Not sending to Multiple Number #20

Closed enderjones closed 3 years ago

enderjones commented 4 years ago

While attempting to send the SMS to multiple phone numbers, it only sends the SMS to the second number after the comma.

TheKoola commented 4 years ago

Looks like the for number loop in line 102 loops over just tonumber = phonenumbers.format... grabbing only the end phone number after the last comma. Lines 104 to 132 need to be indented to be included in the loop.
Also, the return True statements need to be removed from within the loop, the if snapshot: should probably have an else: and the return False should probably be removed at the end of the function definition:

        for number in self._settings.get(['recipient_number']).split(','):
            tonumber = phonenumbers.format_number(phonenumbers.parse(number, 'US'), phonenumbers.PhoneNumberFormat.E164)
            tags = {
                'filename': filename,
                'elapsed_time': elapsed_time,
                'printer_name': self._settings.get(["printer_name"])
            }
            message = self._settings.get(["message_format", "body"]).format(**tags)

            client = TwilioRestClient(self._settings.get(['account_sid']), self._settings.get(['auth_token']))
            if snapshot:
                try:
                    client.messages.create(to=tonumber, from_=fromnumber, body=message, media_url=snapshot)
                except Exception as e:
                    # report problem sending sms
                    self._logger.exception("SMS notification error: %s" % (str(e)))
                    return False
                else:
                    # report notification was sent
                    self._logger.info("Print notification sent to %s" % (self._settings.get(['recipient_number'])))
            else:
                try:
                    client.messages.create(to=tonumber, from_=fromnumber, body=message)
                except Exception as e:
                    # report problem sending sms
                    self._logger.exception("SMS notification error: %s" % (str(e)))
                    return False
                else:
                    # report notification was sent
                    self._logger.info("Print notification sent to %s" % (self._settings.get(['recipient_number'])))

    def _process_snapshot(self, snapshot_path, pixfmt="yuv420p"):

I fixed it on my installation and it sends separate texts to all the phone numbers now.

taxilian commented 4 years ago

Pull requests appreciated...

Sent from my phone

On May 6, 2020, at 08:21, TheKoola notifications@github.com wrote:



Looks like the for number loop in line 102 https://github.com/taxilian/OctoPrint-Twilio/blob/0a60c50c17ca4643cec6acc8c7ea14d0f46e3dd2/octoprint_smsnotifier/__init__.py#L102 loops over just tonumber = phonenumbers.format... grabbing only the end phone number after the last comma. Lines 104 to 132 need to be indented to be included in the loop. Also, the return True statements need to be removed from within the loop, the if snapshot: should probably have an else: and the return False should probably be removed at the end of the function definition:

    for number in self._settings.get(['recipient_number']).split(','):
        tonumber =

phonenumbers.format_number(phonenumbers.parse(number, 'US'), phonenumbers.PhoneNumberFormat.E164) tags = { 'filename': filename, 'elapsed_time': elapsed_time, 'printer_name': self._settings.get(["printer_name"]) } message = self._settings.get(["message_format", "body"]).format(**tags)

        client =

TwilioRestClient(self._settings.get(['account_sid']), self._settings.get(['authtoken'])) if snapshot: try: client.messages.create(to=tonumber, from=fromnumber, body=message, media_url=snapshot) except Exception as e:

report problem sending sms

                self._logger.exception("SMS notification error:

%s" % (str(e))) return False else:

report notification was sent

                self._logger.info("Print notification sent to %s"

% (self._settings.get(['recipientnumber']))) else: try: client.messages.create(to=tonumber, from=fromnumber, body=message) except Exception as e:

report problem sending sms

                self._logger.exception("SMS notification error:

%s" % (str(e))) return False else:

report notification was sent

                self._logger.info("Print notification sent to %s"

% (self._settings.get(['recipient_number'])))

def _process_snapshot(self, snapshot_path, pixfmt="yuv420p"):

I fixed it on my installation and it sends separate texts to all the phone numbers now.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/taxilian/OctoPrint-Twilio/issues/20#issuecomment-624675925, or unsubscribe https://github.com/notifications/unsubscribe-auth/AABWYTWZD5MRNFCYZN7K6DDRQFXADANCNFSM4MZG5IBQ .

enderjones commented 4 years ago

@TheKoola I will attempt this on my copy, thank you!

TheKoola commented 4 years ago

Created pull request #21...

shadycuz commented 3 years ago

@TheKoola After we get the PR merged for python3 compatibility. I will write some unit tests for this plugin, including ones for this bug. I will probably do that in your branch and then we can get your fix merged and released. Should be done tomorrow but if not then by next Sunday for sure =) Sorry for such a long delay.

TheKoola commented 3 years ago

@shadycuz sounds good - thanks. Fix has been stable for me.

TheKoola commented 3 years ago

@shadycuz Thanks -- forgot to set up correct permissions on my fork but thanks for the changes. Looks great to me.

shadycuz commented 3 years ago

Released https://pypi.org/project/OctoPrint-Twilio/