Skip to content

Refactor and optimize Ethernet library #357

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 2 commits into from
Closed

Refactor and optimize Ethernet library #357

wants to merge 2 commits into from

Conversation

rowingdude
Copy link

Using const liberally so that the compiler can try to optimize this down a bit more.

I put in a couple nullptr checks to help stability as I've had my controller crash, hopefully this helps someone.

I created a configure_static_ip function so that int CEthernet::begin ... can remain function specific.

Set ni to a nullptr to avoid any potential "use after free" issues.

Code cleanup, standardize formatting, added nullptr checks to help stability, reorganized to eliminate local variables to save stack space.
@CLAassistant
Copy link

CLAassistant commented Aug 6, 2024

CLA assistant check
All committers have signed the CLA.

Copy link
Contributor

@JAndrassy JAndrassy left a comment

Choose a reason for hiding this comment

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

could you please move configureStaticIP bellow the begin which had that code so it is compared in the diff

@rowingdude
Copy link
Author

Yes, sorry. I got side-tracked adding those annoying /------ ... --/s

Moved function `configureStaticIP` to the end for (hopefully) proper diff alignment.
@rowingdude
Copy link
Author

I will go hunt through the libraries/Wifi/Examples and update as needed to get things moving. I feel like I'm going to end up porting all of the newer WiFi code over to Arduino.

@JAndrassy
Copy link
Contributor

JAndrassy commented Aug 7, 2024

you forgot to commit Ethernet.h?

and please try to eliminate unnecessary differences
https://github.com/arduino/ArduinoCore-renesas/pull/357/files

@rowingdude
Copy link
Author

rowingdude commented Aug 7, 2024

I will do that, there is no "Ethernet.h" in the original directory, so I did not think to include it.
However, thank you for your feedback, these big projects are new to me and I'm learning as I go.

@rowingdude rowingdude closed this Aug 7, 2024
@rowingdude
Copy link
Author

I'm going to close this until I work out the bugs. Thank you, initially my local repo didn't have Ethernet cloned into it and I was going off the web version, however, with the new update, I cannot submit this as is. Cheers

@per1234 per1234 changed the title Update Ethernet.cpp Refactor and optimize Ethernet library Aug 7, 2024
@per1234 per1234 added type: imperfection Perceived defect in any part of project type: enhancement Proposed improvement status: in progress Work is in progress on this topic: code Related to content of the project itself labels Aug 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: in progress Work is in progress on this topic: code Related to content of the project itself type: enhancement Proposed improvement type: imperfection Perceived defect in any part of project
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants