Skip to content
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
Merged

Normalize MAC addresses #16916

merged 2 commits into from Nov 6, 2018

Conversation

balloob
Copy link
Member

@balloob 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.

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))
Copy link
Member

@Kane610 Kane610 Sep 27, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 :)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

@balloob
Copy link
Member Author

balloob commented Nov 6, 2018

Updated PR to handle all notional conventions

@pvizeli pvizeli merged commit bde02af into dev Nov 6, 2018
@ghost ghost removed the in progress label Nov 6, 2018
@pvizeli pvizeli deleted the dev-reg-format-mac branch November 6, 2018 15:33
@balloob balloob mentioned this pull request Nov 29, 2018
@home-assistant home-assistant locked and limited conversation to collaborators Feb 5, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants