stm32duino / STM32Ethernet

Arduino library to support Ethernet for STM32 based board
151 stars 42 forks source link

Ethernet.MACAddress should be a getter, not setter #81

Open JAndrassy opened 8 months ago

JAndrassy commented 8 months ago

Ethernet.MACAddress should be a getter, not setter

https://github.com/arduino-libraries/Ethernet/blob/39103da0e1bc569023625ee4693272773397dbb6/src/Ethernet.cpp#L147-L153

void EthernetClass::MACAddress(uint8_t *mac_address)
{
    SPI.beginTransaction(SPI_ETHERNET_SETTINGS);
    W5100.getMACAddress(mac_address);
    SPI.endTransaction();
}
sstaub commented 8 months ago

I'm confused, what is the problem for this library? You mention the Arduino Ethernet library which uses the WIZnet chips.

JAndrassy commented 8 months ago

in this library it is a setter. that is wrong

sstaub commented 8 months ago

Have a look to the documentation. The implementation is different to the original Arduino library. This is necessary to override builtin mac address.

fpistm commented 8 months ago

Agreed with @sstaub . Getter is available with different prototype which is more clear: https://github.com/stm32duino/STM32Ethernet/blob/cd50f45d53fefd1b947b2a2552f672f328321a3f/src/STM32Ethernet.cpp#L136C1-L136C1

JAndrassy commented 8 months ago

isn't Arduino about using same functions on different hardware? at least functions with the same name should do the same thing.

Arduino Ethernet libraries have MACAddress and WiFi libraries have macAddress as a getter, as a way to read the MAC address. I few libraries have setMACAddress, but usually the way to set the MAC address in Ethernet library is the first parameter of begin

A few months back I stared to write down all my experience with creating and maintaining Arduino networking libraries, because new libraries appear and have the same mistakes I try to eliminate in old libraries. But writing the Guide let to necessity of defining the networking API by analyzing the significant libraries. I found many often simple differences so I started to do PR in all repositories. Then I wrote test sketches for basic functions and those found more errors in implementations than I would expect (even in my libraries.). So more PR.

https://github.com/JAndrassy/Arduino-Networking-API