Skip to content

Add W5100S chip #48

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

Closed
wants to merge 3 commits into from
Closed

Add W5100S chip #48

wants to merge 3 commits into from

Conversation

min-hs
Copy link
Contributor

@min-hs min-hs commented Nov 17, 2021

In the existing Adafurit_wiznet5k library, only W5500 was recognized.
I revised the library to make W5100S possible. Related examples can also be found in the link below.

Copy link
Member

@brentru brentru left a comment

Choose a reason for hiding this comment

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

This is a great pull request, I have a few minor questions/changes.

The PR is also failing at https://github.com/adafruit/Adafruit_CircuitPython_Wiznet5k/runs/4235403094?check_suite_focus=true

We have a guide about checking your pull request's code on https://learn.adafruit.com/improve-your-code-with-pylint/check-your-code-with-pre-commit, please follow this guide and ask me any questions if you need help.

@brentru brentru mentioned this pull request Nov 17, 2021
@min-hs min-hs requested a review from brentru November 18, 2021 03:49
@brentru
Copy link
Member

brentru commented Nov 19, 2021

@min-hs The PR is still failing at https://github.com/adafruit/Adafruit_CircuitPython_Wiznet5k/runs/4246378618?check_suite_focus=true

We have a guide about checking your pull request's code on https://learn.adafruit.com/improve-your-code-with-pylint/check-your-code-with-pre-commit, please follow this guide and ask me any questions if you need help.

I'll merge in once it passes, thanks for making the requested changes.

@min-hs
Copy link
Contributor Author

min-hs commented Nov 23, 2021

We're trying to use W5100 additionally with W5100S, so if possible, we'll promote it together.

Copy link
Member

@anecdata anecdata left a comment

Choose a reason for hiding this comment

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

Tested the PR code on a W5100S-EVB-Pico:
Adafruit CircuitPython 7.1.0-beta.0 on 2021-11-12; Raspberry Pi Pico with rp2040

code.py output:
Wiznet5k Ping Test (no DHCP)
Chip Version: w5100s
MAC Address: ['0x0', '0x1', '0x2', '0x3', '0x4', '0x5']
My IP address is: 192.168.1.100

Nice!

@brentru
Copy link
Member

brentru commented Nov 29, 2021

@min-hs The PR is still failing at https://github.com/adafruit/Adafruit_CircuitPython_Wiznet5k/runs/4246378618?check_suite_focus=true

We have a guide about checking your pull request's code on https://learn.adafruit.com/improve-your-code-with-pylint/check-your-code-with-pre-commit, please follow this guide and ask me any questions if you need help.

I'll merge in once it passes Actions.

@AdamCummick
Copy link
Contributor

I was able to make the changes for black and pylint. I have everything passing pre-commit checks except reuse - I'm unfamiliar with it, so I'm not sure what the traceback is trying to tell me.

reuse....................................................................Failed
- hook id: reuse
- exit code: 1

Traceback (most recent call last):
  File "/home/adamc/.cache/pre-commit/repo7nvm6b2r/py_env-python3/bin/reuse", line 5, in <module>
    from reuse._main import main
  File "/home/adamc/.cache/pre-commit/repo7nvm6b2r/py_env-python3/lib/python3.6/site-packages/reuse/_main.py", line 14, in <module>
    from . import (
  File "/home/adamc/.cache/pre-commit/repo7nvm6b2r/py_env-python3/lib/python3.6/site-packages/reuse/download.py", line 17, in <module>
    from ._util import (
  File "/home/adamc/.cache/pre-commit/repo7nvm6b2r/py_env-python3/lib/python3.6/site-packages/reuse/_util.py", line 26, in <module>
    from debian.copyright import Copyright
  File "/home/adamc/.cache/pre-commit/repo7nvm6b2r/py_env-python3/lib/python3.6/site-packages/debian/copyright.py", line 57, in <module>
    from debian import deb822
  File "/home/adamc/.cache/pre-commit/repo7nvm6b2r/py_env-python3/lib/python3.6/site-packages/debian/deb822.py", line 309, in <module>
    from debian._util import (
  File "/home/adamc/.cache/pre-commit/repo7nvm6b2r/py_env-python3/lib/python3.6/site-packages/debian/_util.py", line 15, in <module>
    """
AttributeError: 'TypeVar' object attribute '__doc__' is read-only

@FoamyGuy
Copy link
Contributor

FoamyGuy commented Jan 7, 2022

@AdamCummick reuse is used for enforcing licensing on all of the published code and files (files will fail the reuse check if they do not reference a valid license.

However the specific issue that you posted the stacktrace for is actually a bit of a red herring I think. The root cause of that issue is python 3.6. I'm not up on the specifics but something changed at some point within the debian builds of python that made them start raising that error for us sometimes. The fix generally has been to update the actions files to use a CI container with python 3.7 at minimum. If you have the option to try running the checks with python 3.7 or greater give that a shot I think it should get you past that error.

@AdamCummick
Copy link
Contributor

Thanks @FoamyGuy , all good now.

Since this isn't my fork, how should I submit the changes?

@brentru
Copy link
Member

brentru commented Jan 10, 2022

@AdamCummick You may try opening a new PR

@FoamyGuy
Copy link
Contributor

@AdamCummick yep new PR is probably best. You can start your branch from min-hs/main branch to preserve the commits from this PR.

@brentru
Copy link
Member

brentru commented Jan 10, 2022

Going with #49, closing..

@brentru brentru closed this Jan 10, 2022
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.

5 participants