Skip to content

boards/arduino*: Added/refactored params for W5100#10844

Merged
miri64 merged 1 commit intoRIOT-OS:masterfrom
maribu:ethernet_shield
Jan 24, 2019
Merged

boards/arduino*: Added/refactored params for W5100#10844
miri64 merged 1 commit intoRIOT-OS:masterfrom
maribu:ethernet_shield

Conversation

@maribu
Copy link
Copy Markdown
Member

@maribu maribu commented Jan 22, 2019 •
edited
Loading

Contribution description

  • ATmega based boards: Added parameter for W5100 to board_common.h
  • Arduino Due based boards: Use board.h to override W5100_PARAM_CS and W5100_PARAM_EVT instead of providing an adapted copy of w5100_params.h

Testing procedure

Running USEMODULE=w5100 BOARD=arduino-mega2560 make in examples/gnrc_minimal should successfully build and when flashed should respond to ping6. (The example will print the IPv6 address over UART).

The same should still work for arduino-due.

Issues/PRs references

None.

@maribu
boards/arduino*: Added/refactored params for W5100
d2c8eab 
- ATmega based boards: Added parameter for W5100 to board_common.h
- Arduino Due based boards: Use board.h instead of providing w5100_params.h
@maribu maribu added Type: enhancement The issue suggests enhanceable parts / The PR enhances parts of the codebase / documentation Area: network Area: Networking Type: cleanup The issue proposes a clean-up / The PR cleans-up parts of the codebase / documentation labels Jan 22, 2019
@maribu maribu added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Jan 22, 2019
@PeterKietzmann PeterKietzmann added Reviewed: 1-fundamentals The fundamentals of the PR were reviewed according to the maintainer guidelines Reviewed: 2-code-design The code design of the PR was reviewed according to the maintainer guidelines Reviewed: 4-code-style The adherence to coding conventions by the PR were reviewed according to the maintainer guidelines labels Jan 22, 2019
@PeterKietzmann
Copy link
Copy Markdown
Member

PeterKietzmann commented Jan 22, 2019

Code-wise makes sense and I set proper labels. Have no hardware to test though.

@maribu
Copy link
Copy Markdown
Member Author

maribu commented Jan 22, 2019

Maybe @kYc0o has an Ethernet Shield and can test this. @kYc0o, do you also have an Arduino Due and mind testing it for both Mega2560 and Due?

@miri64
Copy link
Copy Markdown
Member

miri64 commented Jan 23, 2019

Still works for Arduino Due. However, I didn't check transmitting packets since I don't have my Laptop with me at the moment.

@miri64
Copy link
Copy Markdown
Member

miri64 commented Jan 23, 2019 •
edited
Loading

Ok I tried it now by disconnection my normal workstation from the Internet and doing it with that. Still works as good as it does in master ;-). I thought I was on master, but actually I was on #10412. I faced similar issues as I had there...

@PeterKietzmann
Copy link
Copy Markdown
Member

PeterKietzmann commented Jan 23, 2019

@miri64 this PR is only about pin configuration so any kind of device interaction would be a succeeded test here. Was this the case?

@maribu
Copy link
Copy Markdown
Member Author

maribu commented Jan 23, 2019

I thought I was on master, but actually I was on #10412. I faced similar issues as I had there...

I'm not sure if I understood you correctly. Did this PR lead to the W5100 becoming unreliable (similar to the issues in #10412), but it works reliable in master?

@miri64
Copy link
Copy Markdown
Member

miri64 commented Jan 23, 2019

I'm not sure if I understood you correctly. Did this PR lead to the W5100 becoming unreliable (similar to the issues in #10412), but it works reliable in master?

Yes at least that was what I observed. But I can recheck tomorrow. Maybe some "cosmic rays" were at fault, because I agree with @PeterKietzmann that this shouldn't affect device interaction.

@miri64
Copy link
Copy Markdown
Member

miri64 commented Jan 24, 2019

I can confirm that the unreliabilities also occur on master. So sorry for the noise.

miri64
miri64 approved these changes Jan 24, 2019
View reviewed changes
Copy link
Copy Markdown
Member

@miri64 miri64 left a comment

Choose a reason for hiding this comment

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

Works as good as on master ;-)

@miri64 miri64 added the Reviewed: 3-testing The PR was tested according to the maintainer guidelines label Jan 24, 2019
@miri64
Copy link
Copy Markdown
Member

miri64 commented Jan 24, 2019

Doc is ok under our current practices, but might lead to problems with later Doxygen versions (see #10815)

@miri64 miri64 added the Reviewed: 5-documentation The documentation details of the PR were reviewed according to the maintainer guidelines label Jan 24, 2019
@miri64 miri64 merged commit fbe4363 into RIOT-OS:master Jan 24, 2019
@miri64 miri64 mentioned this pull request Jan 24, 2019
Merged
@aabadie aabadie added this to the Release 2019.01 milestone Jan 25, 2019
@maribu maribu deleted the ethernet_shield branch March 8, 2019 16:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@miri64 miri64 miri64 approved these changes

@haukepetersen haukepetersen Awaiting requested review from haukepetersen

Assignees

No one assigned

Labels

Area: network Area: Networking CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Reviewed: 1-fundamentals The fundamentals of the PR were reviewed according to the maintainer guidelines Reviewed: 2-code-design The code design of the PR was reviewed according to the maintainer guidelines Reviewed: 3-testing The PR was tested according to the maintainer guidelines Reviewed: 4-code-style The adherence to coding conventions by the PR were reviewed according to the maintainer guidelines Reviewed: 5-documentation The documentation details of the PR were reviewed according to the maintainer guidelines Type: cleanup The issue proposes a clean-up / The PR cleans-up parts of the codebase / documentation Type: enhancement The issue suggests enhanceable parts / The PR enhances parts of the codebase / documentation

Projects

None yet

Milestone

Release 2019.01

Development

Successfully merging this pull request may close these issues.

4 participants

@maribu @PeterKietzmann @miri64 @aabadie