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

Fixed issue with passing inverted bits keyword arguments #7

Merged
merged 3 commits into from Jan 14, 2020

Conversation

@bmeisels
Copy link
Contributor

bmeisels commented Jan 6, 2020

When a user passes in black_bits_inverted or color_bits_inverted the IL0373 initializer will pass the keyword argument twice to the displayio.EPaperDisplay initializer rwhich raises an exception ('TypeError: extra keyword arguments given').

The fix is to swap dict.get for dict.pop

@ladyada ladyada requested a review from tannewt Jan 6, 2020
@bmeisels

This comment has been minimized.

Copy link
Contributor Author

bmeisels commented Jan 7, 2020

@tannewt i just noticed that if swap_rams is true that color_bits_inverted / black_bits_inverted are referring to the wrong keyword args. I will add a fix for that too.

@makermelissa

This comment has been minimized.

Copy link
Contributor

makermelissa commented Jan 7, 2020

I think that's on purpose because they're being swapped.

@bmeisels

This comment has been minimized.

Copy link
Contributor Author

bmeisels commented Jan 7, 2020

I thought the swap was referring to the actual commands and not to the inverted option.
If i am wrong though ill remove the last commit.

@makermelissa

This comment has been minimized.

Copy link
Contributor

makermelissa commented Jan 7, 2020

Let's see what @tannewt says.

@tannewt

This comment has been minimized.

Copy link
Collaborator

tannewt commented Jan 7, 2020

Thanks for the fix! I agree that swapping black and color ram args with swap_rams is the right approach. The two examples that use swap_rams don't set either so it is safe to change.

Would you mind adding parameter comments into the class comment? I should have done it when I added the driver.

@bmeisels

This comment has been minimized.

Copy link
Contributor Author

bmeisels commented Jan 9, 2020

@tannewt I added documentation.
I think it would be a good idea to eventually make the kwargs explicit.

Copy link
Collaborator

tannewt left a comment

Thanks for the doc! One question about it.

adafruit_il0373.py Outdated Show resolved Hide resolved
@bmeisels bmeisels force-pushed the bmeisels:kwargs_fix branch from 33aa42e to 6fe04eb Jan 11, 2020
Copy link
Collaborator

tannewt left a comment

Ok, great! Would you mind filing an issue on CircuitPython to support R? It should be easy to add.

@tannewt tannewt merged commit 01ce815 into adafruit:master Jan 14, 2020
1 check passed
1 check passed
test
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.