-
Notifications
You must be signed in to change notification settings - Fork 266
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
Fix issue #140, #105 and #151 #152
base: master
Are you sure you want to change the base?
Conversation
masterx1981
commented
Dec 26, 2020
•
edited
Loading
edited
- Fix issue Register addresses for W5500 are not correct #140 redefining registry and using the correct ones
- Add support for industruino D21G ethernet module (max speed 4mhz)
- Fix issue icmpPing missing member(functions) #105 adding icmp ping support
- Fix issue Overflow warning in htons() #151 using Habazut fix
Addedd last few missing registers for W5500
Merged the ethernet library with the ICMPPing library
Addeed automatic spi setting for industruino
From the W5500 IPRAW documentation (http://www.wizwiki.net/wiki/lib/exe/fetch.php/products:w5500:w5500_ap_ipraw_v100e.pdf): 'After initialization of W5500, the Ping Reply is processed automatically. However, be aware that the Hardwired Ping Reply Logic is disabled if ICMP is opened as SOCKET n in IPRAW mode.'
Fix async ping
Merged some ESP8266 code from the main ERthernet lib
Merged the ifndef htons section from main branch
@masterx1981 your additional commits go into the PR |
Sorry i not know Github very well, the CLA warns me that @felmue not have signed tge agreement so the PR seem locked |
Hello guys I didn't realize you're waiting on me signing the CLA. Sorry about that. Thanks |
No problem, i've noticed it yesterday... i've merged some things of the main branch to our library (minor changes), if they want to merge our version to the main one, i think that it's better. |
maybe check again what you have in this PR. files changed only with commented out debugging. |
Sorry i not understand. I see all the differences from my fork to the basic fork. So, some debug commented out, included some files for the icmp support, corrected registry definitions for w5500, and minor other things. |
@masterx1981 every commit you push to the For this reason, you must dedicate the Please remove any unrelated changes from this pull request. You might find Git's interactive rebase feature to be useful for this purpose. I'll share a overview of that approach:
Please let me know if you have any questions. |
Ooohhh, thanks for the explanation! |
Yes please. Atomic pull requests which propose only one distinct change set are best practices. This makes it easier for interested parties to review. It also avoids the possibility of a "poison pill" situation where acceptance of a straightforward, non-controversial proposal is blocked due to it being bundled unnecessarily with a separate proposal that is difficult to evaluate, or not considered beneficial by all interested parties. |
And, isn't better leave the master branch with all the mods, and create pull request on the branches with every single mod? |
I wouldn't worry about this pull request being from the It is helpful to use a descriptively named branch for each pull request so that you will get a hint at the purpose of each branch from the name while you are navigating your repository. But as far as Arduino is concerned, it makes no difference what the branch you submitted the pull request from is named because the pull request itself should describe the proposal well enough. |
Wouldn't be better for who want to use my library if the master branch is the one with all the mods applied? As i see, the main ethernet library have a lot of uncommited requests, and can be a long wait for have them merged. |
It is up to you how you want to maintain your fork. Some would mirror the Arduino branches and create a dedicated branch for distribution of modified code to users (perhaps even making that one the default branch). Others would use As I mentioned above, the subject of which branch of a fork a PR is submitted from is moot from the perspective of the maintenance of Arduino's own library in this repository. |
Hi everyone, |