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 regexp validation allowing Twilio notifications to use Sender ID instead of phone number #19644

Merged
merged 2 commits into from Dec 30, 2018

Conversation

Projects
None yet
4 participants
@psvanstrom
Copy link
Contributor

psvanstrom commented Dec 29, 2018

Description:

Twilio now supports alphanumerical sender IDs to be used when sending SMS, see https://support.twilio.com/hc/en-us/articles/223181348-Getting-started-with-Alphanumeric-Sender-ID for more information. Sender IDs are available for all paid Twilio accounts and are supported in most countries.

The upside of using sender IDs instead of phone numbers is that you don't have to subscribe to a phone number, the only cost when sending SMS messages will be the actual fee for the message.

This PR updates the regular expression validating the from_number configuration parameter so that it allows both phone numbers as well as sender IDs, by applying the rules enforced by Twilio:

Alphanumeric Sender ID supports up to 11 characters from the following categories:
Upper-case letters A-Z
Lower-case letters a-z
Numbers 0-9
Spaces
Your ID must include at least one letter; It cannot be comprised of only numbers. Non-ASCII special characters and punctuation are not allowed.

Example entry for configuration.yaml:

notify:
  - name: NOTIFIER_NAME
    platform: twilio_sms
    from_number: MyHouse

Checklist:

  • The code change is tested and works locally.
  • Local tests pass with tox. Your PR cannot be merged unless tests pass
  • There is no commented out code in this PR.

If user exposed functionality or configuration variables are added/changed:

Pull request in home-assistant.io with documentation (if applicable): home-assistant/home-assistant.io#7980

psvanstrom added a commit to psvanstrom/home-assistant.io that referenced this pull request Dec 29, 2018

Update Twilio SMS component with Sender ID
Update the SMS configuration documentation with Sender ID as proposed in this PR: home-assistant/home-assistant#19644

@psvanstrom psvanstrom referenced this pull request Dec 29, 2018

Merged

Update Twilio SMS component with Sender ID #7980

2 of 2 tasks complete
@fabaff

This comment has been minimized.

Copy link
Member

fabaff commented Dec 29, 2018

@psvanstrom, could you please update the requirement in a different PR as you have a working Twilio setup to test it?

Thanks

@psvanstrom

This comment has been minimized.

Copy link
Contributor

psvanstrom commented Dec 29, 2018

@fabaff I'd love to, but I don't really understand what you mean with "update the requirement"? 😄 This is my first PR in home-assistant so I might have missed something obvious?

@fabaff

This comment has been minimized.

Copy link
Member

fabaff commented Dec 29, 2018

The Twilio component (twilio/__init__.py) has REQUIREMENTS = ['twilio==6.19.1']. This defines the release of the module we are using. The latest release of twilio is 6.22.0. It's always good to test the upgrades of the Python modules and a real test requires that a person has a working setup and is using the integration.

Change the requirement (REQUIREMENTS = ['twilio==6.22.0']) and run script/gen_requirements_all.py.

@psvanstrom

This comment has been minimized.

Copy link
Contributor

psvanstrom commented Dec 30, 2018

Ah ok, will do that! Is it ok if I do that in this PR or should it be in a separate one?

@fabaff

This comment has been minimized.

Copy link
Member

fabaff commented Dec 30, 2018

Please create a new PR. Easier to keep track of changes if there is one per PR and the IPv6 support seems to be independent from an upgrade to the Twilio Python module.

@fabaff

fabaff approved these changes Dec 30, 2018

@fabaff fabaff merged commit 18d36e0 into home-assistant:dev Dec 30, 2018

5 checks passed

Hound No violations found. Woof!
WIP Legacy commit status override — see details
Details
cla-bot Everyone involved has signed the CLA
continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls Coverage decreased (-0.008%) to 93.062%
Details

@wafflebot wafflebot bot removed the in progress label Dec 30, 2018

@psvanstrom psvanstrom deleted the psvanstrom:allow-sender-id-for-twilio-sms branch Dec 30, 2018

fabaff added a commit to home-assistant/home-assistant.io that referenced this pull request Dec 31, 2018

Update Twilio SMS component with Sender ID (#7980)
Update the SMS configuration documentation with Sender ID as proposed in this PR: home-assistant/home-assistant#19644

mxworm added a commit to mxworm/home-assistant that referenced this pull request Jan 3, 2019

Merge branch 'dev' into current
* dev:
  Adds ability to calibrate temperature for BME680 (home-assistant#19684)
  Bumping aioasuswrt version to 1.1.17 (home-assistant#19714)
  Bump pyotgw to 0.4b1 (home-assistant#19715)
  Envisalink pgm (home-assistant#19499)
  Update pyhomematic 0.1.54 + small fixes (home-assistant#19667)
  Add ness alarm control panel using nessclient (home-assistant#18463)
  Add support for color_temp_command_template in MQTT light component (home-assistant#19675)
  Improve rflink coverage (home-assistant#19596)
  Luftdaten traceback (home-assistant#19666)
  Fix london_underground issue (home-assistant#19642)
  Add IDTECK proximity card component (home-assistant#18309)
  Fix homekit_controller pairing regression (home-assistant#19654)
  Fix error in got_connected for remote.harmony (home-assistant#19662)
  Fix exception checking for next dublin bus (home-assistant#19663)
  Added regexp validation allowing Twilio notifications to use Sender ID instead of phone number (home-assistant#19644)
  pytraccar version bump (home-assistant#19659)
  Suppress traceback if network is not available

@balloob balloob referenced this pull request Jan 10, 2019

Merged

0.85.0 #19897

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment