Skip to content

Ethernet - standard MACAddress getter with a parameter #214

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
Merged
merged 1 commit into from
Dec 20, 2023
Merged

Ethernet - standard MACAddress getter with a parameter #214

merged 1 commit into from
Dec 20, 2023

Conversation

JAndrassy
Copy link
Contributor

@JAndrassy JAndrassy commented Dec 19, 2023 •
edited
Loading

MACAddress getter as in other Ethernet library

overview of legacy Ethernet specific getters and setters in libraries

@aentinger
Copy link
Contributor

Why not

void CEthernet::MACAddress(uint8_t *mac) {
  CLwipIf::getInstance().getMacAddress(NI_ETHERNET, mac);
}

?

@JAndrassy
Copy link
Contributor Author

JAndrassy commented Dec 20, 2023 •
edited
Loading

Why not?

so the mac_address field is filled too.
but now I see the field is not used anywhere, it is only for the MACAddress function.
I change the PR.
best would be to remove the mac_address field and the MACAddress method version without parameter.
I guess the library doesn't have much users yet.

@aentinger
Copy link
Contributor

I change the PR.

Please do 🙏 🙇 .

@JAndrassy JAndrassy force-pushed the ethernet_macaddress_getter branch from b40a9ad to be5d98f Compare December 20, 2023 11:41
@JAndrassy
Copy link
Contributor Author

modified

aentinger
aentinger approved these changes Dec 20, 2023
View reviewed changes
Copy link
Contributor

@aentinger aentinger left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@aentinger aentinger merged commit 6ee152f into arduino:main Dec 20, 2023
@JAndrassy JAndrassy deleted the ethernet_macaddress_getter branch December 20, 2023 13:57
@per1234 per1234 added type: enhancement Proposed improvement topic: code Related to content of the project itself labels Dec 20, 2023
@JAndrassy JAndrassy mentioned this pull request Dec 23, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@aentinger aentinger aentinger approved these changes

Assignees
No one assigned
Labels
topic: code Related to content of the project itself type: enhancement Proposed improvement
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@JAndrassy @aentinger @per1234