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

Split Address into two groups. #83

Merged
merged 5 commits into from Dec 9, 2017

Conversation

Projects
None yet
3 participants
@encbladexp
Contributor

encbladexp commented Dec 3, 2017

Currently we use Address() for just everything address related. This results in a lot of unstructured code and is not really python like.

This change splits Address() in GroupAddress and PhysicalAddress to honor the specials of these types much better. Later on we should provide more helpers, like is_valid.

After all, we should think about uninitialized Addresses in xknx at all, to avoid potential fuckups as 0 is the current default address in some parts.

This changes breaks compatibility with existing third party software, if they use Address. I think its time to create some CHANGELOG.md for the next release.

encbladexp added some commits Dec 2, 2017

Removed manual test loading at the end of all tests.
This i not needed, as python unittest autodiscovery works very well.
Split Address in two dedicated groups.
This change splits `Address` in `GroupAddress` and `PhysicalAddress`.
@coveralls

This comment has been minimized.

Show comment
Hide comment
@coveralls

coveralls Dec 3, 2017

Coverage Status

Coverage increased (+0.1%) to 80.964% when pulling d78ef48 on encbladexp:splitaddresses into 4a68fdd on XKNX:master.

coveralls commented Dec 3, 2017

Coverage Status

Coverage increased (+0.1%) to 80.964% when pulling d78ef48 on encbladexp:splitaddresses into 4a68fdd on XKNX:master.

@coveralls

This comment has been minimized.

Show comment
Hide comment
@coveralls

coveralls Dec 3, 2017

Coverage Status

Coverage increased (+0.1%) to 80.964% when pulling d78ef48 on encbladexp:splitaddresses into 4a68fdd on XKNX:master.

coveralls commented Dec 3, 2017

Coverage Status

Coverage increased (+0.1%) to 80.964% when pulling d78ef48 on encbladexp:splitaddresses into 4a68fdd on XKNX:master.

@coveralls

This comment has been minimized.

Show comment
Hide comment
@coveralls

coveralls Dec 3, 2017

Coverage Status

Coverage increased (+0.1%) to 80.964% when pulling 8a0ff21 on encbladexp:splitaddresses into 4a68fdd on XKNX:master.

coveralls commented Dec 3, 2017

Coverage Status

Coverage increased (+0.1%) to 80.964% when pulling 8a0ff21 on encbladexp:splitaddresses into 4a68fdd on XKNX:master.

@Julius2342

This comment has been minimized.

Show comment
Hide comment
@Julius2342

Julius2342 Dec 3, 2017

Collaborator

great! thank you! i will review asap!

Collaborator

Julius2342 commented Dec 3, 2017

great! thank you! i will review asap!

@Julius2342 Julius2342 changed the base branch from master to splitaddresses Dec 9, 2017

@Julius2342 Julius2342 merged commit 415e219 into XKNX:splitaddresses Dec 9, 2017

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls Coverage increased (+0.1%) to 80.964%
Details
@Julius2342

This comment has been minimized.

Show comment
Hide comment
@Julius2342

Julius2342 Dec 9, 2017

Collaborator

I merged it to splitaddresses, will do some tests and then merge to dev/master...

Collaborator

Julius2342 commented Dec 9, 2017

I merged it to splitaddresses, will do some tests and then merge to dev/master...

@encbladexp encbladexp deleted the encbladexp:splitaddresses branch Dec 9, 2017

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