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.

Sign up for GitHub

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

Jump to bottom

Update ssd1680_simpletest.py #9

Merged
merged 9 commits into from
Feb 15, 2023
Merged

Update ssd1680_simpletest.py #9

merged 9 commits into from
Feb 15, 2023

Conversation

dave-ct
Copy link
Contributor

@dave-ct dave-ct commented Feb 11, 2023

Added comment on how to change display height for Monochrome version to work correctly

@dave-ct
Update ssd1680_simpletest.py
b8e327a 
Added comment on setting for MonoChrome version .
@dave-ct
Update ssd1680_simpletest.py
68ba28b
@tekktrik tekktrik requested a review from a team February 12, 2023 02:51
tannewt
tannewt reviewed Feb 13, 2023
View reviewed changes
Copy link
Member

@tannewt tannewt left a comment

Choose a reason for hiding this comment

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

Want to add a separate example for the monochrome display?

@dave-ct
Copy link
Contributor Author

dave-ct commented Feb 13, 2023

Want to add a separate example for the monochrome display?

@tannewt Will have a go at the weekend and update this pull request.

@tannewt
Copy link
Member

tannewt commented Feb 14, 2023

Thank you!

@dave-ct dave-ct mentioned this pull request Feb 15, 2023
Closed
@makermelissa
Copy link
Collaborator

makermelissa commented Feb 15, 2023 •
edited
Loading

Want to add a separate example for the monochrome display?

@tannewt Will have a go at the weekend and update this pull request.

I think maybe adjusting the offset by adding a rowstart=5 parameter (possibly it may be colstart instead) to the init code would be the appropriate fix rather than adjusting the height which brings its own set of issues.

@jposada202020
Copy link
Contributor

probably would be a good idea to verify #5 at the same time. sadly, I do not have the hardware to make the tests.

@dave-ct
Adjust colstart to support top/bottom row correction
e181b0a 
Added column_corection argument to ssd1680 to set colstart so can be adjusted in simpletest to fix issue adafruit#10  and can still work with previous sampletest file if needed as only tested on ESP32-S2.
@dave-ct
Copy link
Contributor Author

dave-ct commented Feb 15, 2023

@makermelissa @jposada202020 @tannewt Have made an update to the ssd1680 and simpletest example that works now on both mono and tri colour featherwings. As cannot test on other devices have made clostart default to 1 unless set otherwise so ssd1680 will still work previous simpletest versions.

@dave-ct
Copy link
Contributor Author

dave-ct commented Feb 15, 2023

Not sure why the Formatting checks have failed?

@jposada202020
Copy link
Contributor

dave-ct is because of the formating.

Error: reformatted adafruit_ssd1680.py from (https://github.com/adafruit/Adafruit_CircuitPython_SSD1680/actions/runs/4187808124/jobs/7258138283)

this is easily fixed using pre-commit run --all-files you could find more info here...

https://learn.adafruit.com/improve-your-code-with-pylint/check-your-code-with-pre-commit

Let me know if you need more help :)

@dave-ct
Copy link
Contributor Author

dave-ct commented Feb 15, 2023

@jposada202020 that did the job, checks all passed now. Thanks for the pointer.

@jposada202020
Copy link
Contributor

@dave-ct I will take a look over the weekend, hopefully, thanks again :)

@makermelissa
Copy link
Collaborator

Hi, for the driver, because of the **kwargs in the init function, you should be able to just remove the colstart=1, and just add colstart to the init code in the example and it will automatically pass through. This avoids introducing a new parameter.

Since the code is likely already in use, you should also check if it was in kwargs and default to 1 if not. This way we can have the intended functionality without breaking anything.

makermelissa
makermelissa requested changes Feb 15, 2023
View reviewed changes
Copy link
Collaborator

@makermelissa makermelissa left a comment

Choose a reason for hiding this comment

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

Here's the changes I suggested.

adafruit_ssd1680.py Outdated Show resolved Hide resolved
adafruit_ssd1680.py Outdated Show resolved Hide resolved
adafruit_ssd1680.py Outdated Show resolved Hide resolved
examples/ssd1680_simpletest.py Outdated Show resolved Hide resolved
dave-ct and others added 5 commits February 15, 2023 22:07
@dave-ct @makermelissa
Update adafruit_ssd1680.py
2075bbb 
Co-authored-by: Melissa LeBlanc-Williams <melissa@adafruit.com>
@dave-ct @makermelissa
Update adafruit_ssd1680.py
f85db07 
Co-authored-by: Melissa LeBlanc-Williams <melissa@adafruit.com>
@dave-ct @makermelissa
Update adafruit_ssd1680.py
8ec9277 
Co-authored-by: Melissa LeBlanc-Williams <melissa@adafruit.com>
@dave-ct @makermelissa
Update examples/ssd1680_simpletest.py
3fc67ff 
Co-authored-by: Melissa LeBlanc-Williams <melissa@adafruit.com>
@dave-ct
Update ssd1680_simpletest.py
9151c72 
Corrected guidance comment
@dave-ct
Copy link
Contributor Author

dave-ct commented Feb 15, 2023

@makermelissa much cleaner way than what I did. Tested fine on my devices.

makermelissa
makermelissa approved these changes Feb 15, 2023
View reviewed changes
Copy link
Collaborator

@makermelissa makermelissa left a comment

Choose a reason for hiding this comment

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

Looks great. thanks.

@makermelissa makermelissa merged commit dfff3b6 into adafruit:main Feb 15, 2023
@tannewt
Copy link
Member

tannewt commented Feb 16, 2023

If different devices need a different colstart setting, then it'd be ideal to have separate examples for them. That's easier than reading comments and changing code.

@makermelissa
Copy link
Collaborator

Oops, yeah. I guess I merged too soon.

@jposada202020 jposada202020 mentioned this pull request Feb 18, 2023
Closed
adafruit-adabot added a commit to adafruit/Adafruit_CircuitPython_Bundle that referenced this pull request Feb 28, 2023
@adafruit-adabot
Automated update by Adabot (adafruit/adabot@2415fa6)
0300f10 
Updating https://github.com/adafruit/Adafruit_CircuitPython_DisplayIO_SSD1306 to 1.6.0 from 1.5.11:
  > Merge pull request adafruit/Adafruit_CircuitPython_DisplayIO_SSD1306#31 from dglaude/0.49''-64-x-32
  > Add upload url to release action
  > Add .venv to .gitignore

Updating https://github.com/adafruit/Adafruit_CircuitPython_IS31FL3731 to 3.3.7 from 3.3.6:
  > Merge pull request adafruit/Adafruit_CircuitPython_IS31FL3731#53 from EAGrahamJr/fade_52
  > Add upload url to release action
  > Add .venv to .gitignore

Updating https://github.com/adafruit/Adafruit_CircuitPython_PN532 to 2.3.16 from 2.3.14:
  > Merge pull request adafruit/Adafruit_CircuitPython_PN532#63 from caternuson/iss44
  > Merge pull request adafruit/Adafruit_CircuitPython_PN532#62 from caternuson/iss49
  > Add upload url to release action
  > Add .venv to .gitignore

Updating https://github.com/adafruit/Adafruit_CircuitPython_RFM69 to 2.1.14 from 2.1.13:
  > Merge pull request adafruit/Adafruit_CircuitPython_RFM69#44 from jerryneedell/jerryn_size
  > Add upload url to release action
  > Add .venv to .gitignore
  > Update .pylintrc for v2.15.5
  > Fix release CI files
  > Update pylint to 2.15.5
  > Updated pylint version to 2.13.0
  > Switching to composite actions

Updating https://github.com/adafruit/Adafruit_CircuitPython_SSD1675 to 1.1.16 from 1.1.15:
  > Merge pull request adafruit/Adafruit_CircuitPython_SSD1675#14 from jposada202020/updating_example
  > Add upload url to release action
  > Add .venv to .gitignore
  > Update .pylintrc for v2.15.5
  > Fix release CI files
  > Update pylint to 2.15.5
  > Updated pylint version to 2.13.0
  > Switching to composite actions

Updating https://github.com/adafruit/Adafruit_CircuitPython_SSD1680 to 1.0.14 from 1.0.12:
  > Merge pull request adafruit/Adafruit_CircuitPython_SSD1680#12 from jposada202020/adding_breakout_example
  > Merge pull request adafruit/Adafruit_CircuitPython_SSD1680#11 from jposada202020/main
  > Merge pull request adafruit/Adafruit_CircuitPython_SSD1680#9 from dave-ct/128
  > Add upload url to release action
  > Add .venv to .gitignore
  > Update .pylintrc for v2.15.5
  > Fix release CI files
  > Update pylint to 2.15.5
  > Updated pylint version to 2.13.0
  > Switching to composite actions

Updating https://github.com/adafruit/Adafruit_CircuitPython_Touchscreen to 1.2.0 from 1.1.17:
  > Merge pull request adafruit/Adafruit_CircuitPython_Touchscreen#23 from rtwfroody/invert
  > Add upload url to release action
  > Add .venv to .gitignore
  > Update .pylintrc for v2.15.5
  > Fix release CI files
  > Update pylint to 2.15.5
  > Updated pylint version to 2.13.0
  > Switching to composite actions

Updating https://github.com/adafruit/Adafruit_CircuitPython_AirLift to 1.0.9 from 1.0.8:
  > Merge pull request adafruit/Adafruit_CircuitPython_AirLift#8 from glenrobertson/patch-1
  > Add upload url to release action
  > Add .venv to .gitignore
  > Update .pylintrc for v2.15.5
  > Fix release CI files
  > Update pylint to 2.15.5
  > Updated pylint version to 2.13.0
  > Switching to composite actions

Updating https://github.com/adafruit/Adafruit_CircuitPython_LED_Animation to 2.7.0 from 2.6.6:
  > Merge pull request adafruit/Adafruit_CircuitPython_LED_Animation#105 from FoamyGuy/multicolor_comet

Updating https://github.com/adafruit/Adafruit_CircuitPython_MatrixPortal to 3.0.11 from 3.0.10:
  > Merge pull request adafruit/Adafruit_CircuitPython_MatrixPortal#88 from Lnk2past/master
  > Add upload url to release action
  > Add .venv to .gitignore
  > Update .pylintrc for v2.15.5
  > Fix release CI files
  > Update pylint to 2.15.5
  > Updated pylint version to 2.13.0
  > Switching to composite actions

Updating https://github.com/adafruit/Adafruit_CircuitPython_MiniMQTT to 7.3.1 from 7.3.0:
  > Merge pull request adafruit/Adafruit_CircuitPython_MiniMQTT#156 from vladak/suback_var_payload
  > Merge pull request adafruit/Adafruit_CircuitPython_MiniMQTT#153 from vladak/back_off_tests

Updating https://github.com/adafruit/Adafruit_CircuitPython_Bundle/circuitpython_library_list.md to NA from NA:
  > Updated download stats for the libraries
@makermelissa makermelissa mentioned this pull request Oct 2, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tannewt tannewt tannewt left review comments

@makermelissa makermelissa makermelissa approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@dave-ct @tannewt @makermelissa @jposada202020