Network interface MAC address set/get #13936
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
added 6 commits
November 18, 2020 17:17
Add method set_mac_address to set network interface MAC address.
Add support to set and get MAC address from EMAC interface.
Remove unnecessary method: add_ethernet_interface_ns
Remove final keyword to allow inheritance.
Use nsapi_size_t instread os size_t.
ciarmcom
requested review from
JarkkoPaso,
juhhei01,
mikaleppanen,
mikter and
a team
|
@artokin, thank you for your changes. |
juhhei01
approved these changes
Nov 20, 2020
JarkkoPaso
approved these changes
Nov 20, 2020
mikaleppanen
approved these changes
Nov 20, 2020
| * All interfaces are not supporting MAC address set. A call to connect() | ||
| * will fail if MAC address is provided but not possible to use. | ||
| * | ||
| * @param mac_addr Buffer containing the MAC address |
There was a problem hiding this comment.
added "in hexadecimal format"
| * set before calling the interface connect() method. EUI-48 MAC addresses | ||
| * are used for Ethernet while Mesh interface is using 8-byte EUI-64 address. | ||
| * | ||
| * All interfaces are not supporting MAC address set. A call to connect() |
There was a problem hiding this comment.
can't the set_mac_address return an error if it's not supported?
There was a problem hiding this comment.
Not easily as the interface initialisation happens during the connect()-method call.
There was a problem hiding this comment.
If connect fails I think this makes quite inflexible interface as you can't recover from this and can't make a feature to read Mac from Factory tool, but for devices that has HW support for getting MAC it should override the factory tool configuration. So connect should not fail, but trace out an warning
There was a problem hiding this comment.
@mikter , good point!
Connect() failure is now ignored in the correction commit. Would you please review again?
|
|
||
| /** Set the MAC address to the interface. | ||
| * | ||
| * Provided MAC address is set the the network interface. Address must be |
There was a problem hiding this comment.
Suggested change
| * Provided MAC address is set the the network interface. Address must be | |
| * Provided MAC address is set on the network interface. Address must be |
paul-szczepanek-arm
suggested changes
Nov 20, 2020
|
|
||
| nsapi_error_t EmacTestNetworkStack::Interface::set_mac_address(uint8_t *buf, nsapi_size_t buflen) | ||
| { | ||
| return NSAPI_ERROR_OK; |
There was a problem hiding this comment.
Shouldn't this be NSAPI_ERROR_UNSUPPORTED?
-Update method set_mac_address description -Update EMAC test method return value
|
@paul-szczepanek-arm , thanks for your comments. |
paul-szczepanek-arm
approved these changes
Nov 23, 2020
Jenkins CI Test : ❌ FAILEDBuild Number: 3 | 🔒 Jenkins CI Job | 🌐 Logs & ArtifactsCLICK for Detailed Summary
|
artokin
force-pushed
the
mac_address_set_get_mbed_os_master
branch
from
ac517a8 to
0bc2b9e
Compare
-Remove Nanostack::add_ethernet_interface API change -Add get_mac_address to MeshEthernetInterface -Fix compiler warnings by adding overrides
artokin
force-pushed
the
mac_address_set_get_mbed_os_master
branch
from
0bc2b9e to
6a28bce
Compare
A new method has been added to the NetworkInterface. Therfore WICED library needs to be rebuild for ARMC6.
|
CI restarted |
Jenkins CI Test : ❌ FAILEDBuild Number: 4 | 🔒 Jenkins CI Job | 🌐 Logs & ArtifactsCLICK for Detailed Summary
|
|
CI restarted |
Jenkins CI Test : ❌ FAILEDBuild Number: 5 | 🔒 Jenkins CI Job | 🌐 Logs & ArtifactsCLICK for Detailed Summary
|
Jenkins CI Test : ✔️ SUCCESSBuild Number: 6 | 🔒 Jenkins CI Job | 🌐 Logs & ArtifactsCLICK for Detailed Summary
|
0xc0170
approved these changes
Dec 9, 2020
mbedmain
added
release-version: 6.6.0
Release-pending
and removed
release-type: feature
Release-pending
labels
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary of changes
Add method set_mac_address that Application can use to set MAC address to the interface.
Update get_mac_address to return MAC address correctly from EMAC.
This PR is copied from: #13902
Impact of changes
Migration actions required
Documentation
Pull request type
Test results
Reviewers
@mikter , @mikaleppanen , @juhhei01 , @JarkkoPaso