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

Normalize MAC addresses #16916

Merged
merged 2 commits into from Nov 6, 2018

Conversation

Projects
None yet
4 participants
@balloob
Member

balloob commented Sep 27, 2018

Description:

Normalize incoming mac addresses for the device registry.

CC @Kane610

Checklist:

  • The code change is tested and works locally.
  • Local tests pass with tox. Your PR cannot be merged unless tests pass

If the code does not interact with devices:

  • Tests have been added to verify that the new code works.

@balloob balloob requested a review from home-assistant/core as a code owner Sep 27, 2018

@wafflebot wafflebot bot added the in progress label Sep 27, 2018

lower_mac = mac.lower()
if len(lower_mac) == 12:
# no : included
return ':'.join(lower_mac[i:i + 2] for i in range(0, 12, 2))

This comment has been minimized.

@Kane610

Kane610 Sep 27, 2018

Contributor

Wouldn't it be better to strip everything but hexadecimal values from the Mac? There are multiple standards to format the Mac. Also since if one implementation modifies its Mac to follow its own standard the smallest common denominator should be used to map between components. Or specify what format the Mac needs to be in.

This comment has been minimized.

@balloob

balloob Sep 27, 2018

Member

We are normalizing the value here to lowercase and separated by :. If an integration sends us any other format, it will be reformatted to ours, so no need to stick to a common denominator because we specify the common one :)

This comment has been minimized.

@Kane610

Kane610 Sep 27, 2018

Contributor

It only normalise Mac that doesn't have any ':' but doesn't cover Mac that uses '.' or '-' as separators, see https://en.wikipedia.org/wiki/MAC_address

This comment has been minimized.

@balloob

balloob Sep 27, 2018

Member

Okay, I'll strip out any non hexadecimal value and then format it with :?

@OttoWinter OttoWinter referenced this pull request Sep 28, 2018

Merged

Implement base for MQTT device registry integration #16943

4 of 4 tasks complete

balloob added some commits Sep 27, 2018

@balloob balloob force-pushed the dev-reg-format-mac branch from d925068 to c302934 Nov 6, 2018

@balloob

This comment has been minimized.

Member

balloob commented Nov 6, 2018

Updated PR to handle all notional conventions

@pvizeli

pvizeli approved these changes Nov 6, 2018

@pvizeli pvizeli merged commit bde02af into dev Nov 6, 2018

3 of 5 checks passed

continuous-integration/travis-ci/pr The Travis CI build is in progress
Details
continuous-integration/travis-ci/push The Travis CI build is in progress
Details
Hound No violations found. Woof!
WIP ready for review
Details
cla-bot Everyone involved has signed the CLA

@wafflebot wafflebot bot removed the in progress label Nov 6, 2018

@pvizeli pvizeli deleted the dev-reg-format-mac branch Nov 6, 2018

@ehendrix23 ehendrix23 referenced this pull request Nov 6, 2018

Merged

Remove turn_on and turn_off feature for clients #18234

2 of 2 tasks complete

zxdavb added a commit to zxdavb/home-assistant that referenced this pull request Nov 13, 2018

Normalize MAC addresses (home-assistant#16916)
* Normalize MAC addresses

* Handle all mac formats

@balloob balloob referenced this pull request Nov 29, 2018

Merged

0.83 #18776

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