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

Higher level interface to BLNET #16

Merged
merged 6 commits into from
Jan 5, 2019
Merged

Higher level interface to BLNET #16

merged 6 commits into from
Jan 5, 2019

Conversation

henfri
Copy link
Contributor

@henfri henfri commented Dec 10, 2018

second try of higher level interface

second try of higher level interface
@pep8speaks
Copy link

pep8speaks commented Dec 10, 2018

Hello @henfri! Thanks for updating the PR.

Line 8:80: E501 line too long (88 > 79 characters)
Line 22:1: W293 blank line contains whitespace
Line 23:80: E501 line too long (92 > 79 characters)
Line 33:80: E501 line too long (84 > 79 characters)
Line 61:80: E501 line too long (91 > 79 characters)
Line 65:80: E501 line too long (91 > 79 characters)
Line 119:80: E501 line too long (83 > 79 characters)
Line 138:80: E501 line too long (86 > 79 characters)
Line 142:80: E501 line too long (91 > 79 characters)
Line 143:80: E501 line too long (97 > 79 characters)
Line 144:79: W291 trailing whitespace
Line 145:80: E501 line too long (85 > 79 characters)
Line 151:39: E701 multiple statements on one line (colon)
Line 160:1: W293 blank line contains whitespace
Line 162:80: E501 line too long (95 > 79 characters)
Line 166:80: E501 line too long (94 > 79 characters)
Line 170:80: E501 line too long (94 > 79 characters)
Line 172:1: W293 blank line contains whitespace
Line 174:80: E501 line too long (94 > 79 characters)
Line 178:80: E501 line too long (93 > 79 characters)
Line 180:1: W293 blank line contains whitespace
Line 182:17: E225 missing whitespace around operator
Line 182:80: E501 line too long (91 > 79 characters)
Line 184:1: W293 blank line contains whitespace
Line 185:1: W293 blank line contains whitespace
Line 186:1: W293 blank line contains whitespace
Line 187:5: E303 too many blank lines (3)

Line 29:80: E501 line too long (80 > 79 characters)
Line 59:80: E501 line too long (83 > 79 characters)
Line 64:80: E501 line too long (100 > 79 characters)
Line 69:5: E303 too many blank lines (2)
Line 76:80: E501 line too long (95 > 79 characters)
Line 77:80: E501 line too long (107 > 79 characters)
Line 78:80: E501 line too long (118 > 79 characters)
Line 79:80: E501 line too long (109 > 79 characters)
Line 80:80: E501 line too long (107 > 79 characters)
Line 82:80: E501 line too long (91 > 79 characters)
Line 86:80: E501 line too long (95 > 79 characters)
Line 97:5: E303 too many blank lines (2)
Line 100:80: E501 line too long (83 > 79 characters)
Line 107:80: E501 line too long (84 > 79 characters)

Comment last updated on January 05, 2019 at 23:43 Hours UTC

@nielstron
Copy link
Owner

Neat!

Just thinking about it. Maybe a method "get_digital_mode" would be useful too. And maybe "get_digital" which the returns a tuple of value and mode.

Also unittests would be great to test these functions against real life data.

@nielstron nielstron changed the base branch from master to dev December 10, 2018 23:37
@nielstron nielstron changed the title Update blnet.py Higher level interface to BLNET Dec 10, 2018
Copy link
Owner

@nielstron nielstron left a comment

Choose a reason for hiding this comment

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

Please do rework your proposal.

pyblnet/blnet.py Outdated Show resolved Hide resolved
pyblnet/blnet.py Outdated Show resolved Hide resolved
pyblnet/blnet.py Outdated Show resolved Hide resolved
pyblnet/blnet.py Outdated Show resolved Hide resolved
Fixes according to @nielstrons requests/comments
@henfri
Copy link
Contributor Author

henfri commented Dec 28, 2018

Hello,

I updated the file. Please have a look.

Greetings,
Hendrik

@nielstron nielstron merged commit c22aa9a into nielstron:dev Jan 5, 2019
@henfri
Copy link
Contributor Author

henfri commented Jan 6, 2019

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants