New Class MacAddress similar to IPAddress

[mrengineer7777]

Description of Change

This adds new classes MacAddress and MacAddress8. MacAddress allows for better handling of MAC and BSSID addresses. MacAddress8 is for 8-byte EUI-64 as noted in esp_mac.h.

Test scenarios

Tested on Arduino Feather ESP32.

[env]
monitor_speed = 115200
build_flags = 

[env:fthr-esp32ard]
platform = https://github.com/tasmota/platform-espressif32/releases/download/v2.0.2.3/platform-espressif32-2.0.2.3.zip
framework = arduino
board = featheresp32

RC1

No new changes. Bugfixes accepted.

TESTING

See test project here: https://github.com/mrengineer7777/MA_Test.

Related links

Closes #6658.

[github-actions]

Unit Test Results

0 files  0 suites   0s ⏱️
0 tests 0 ✔️ 0 💤 0 ❌

Results for commit 5a0a7d5.

♻️ This comment has been updated with latest results.

[SuGlider]

@mrengineer7777 - Let me know when it is ready.

[mrengineer7777]

@SuGlider Tested working. Test project here: https://github.com/mrengineer7777/MA_Test.
Can you help me with integration into the project? I don't know the proper places to declare the new files. Open to suggestions.

[mrengineer7777]

Looks like I'll need to update CMakeLists.txt if we are happy with the file locations.

[SuGlider]

Looks like I'll need to update CMakeLists.txt if we are happy with the file locations.

Yes, it's just a matter of adding it. CMake/CI needs all files in place. Thanks!

[mrengineer7777]

@SuGlider Updated. Success.

[SuGlider]

@mrengineer7777 Thanks!

I have an initial suggestion:

• Do you mind using Apache License instead, like the one used in https://github.com/espressif/arduino-esp32/blob/master/cores/esp32/HWCDC.h#L1-L13 - we are looking into moving as much as possible to Apache Licensing. But, feel free to decide whatever you want as an author!

[SuGlider]

@mrengineer7777 - Talking to @me-no-dev, he made some nice suggestions also:

1- MacAddress6 should be named just MacAddress, as it is the default size for our SoCs and mostly known. MacAddress8 seems to make a lot of sense with IPv6.
2- @me-no-dev said that you may want to also submit a PR to Arduino main GitHub, so in the future all Arduino platforms around the world will look the same regarding this class. That would be awesome!

I also would suggest some adding to the code - in the review.

[SuGlider]

Very good! Just a few methods to make it closer to IPAddress class :-)

Same changing suggestions from Mac6 to Mac8.

[mrengineer7777]

@SuGlider I have also updated the test project

[mrengineer7777]

Just learned how to squash commits using GitHub Desktop :)

[SuGlider]

@mrengineer7777 | @me-no-dev

Thanks David for adding this class!
I have added all necessary operators and Printable to the classes in order to make it closer to IPAddress Arduino Class.
Let me know if you are OK with it.

[mrengineer7777]

@SuGlider Thanks for adding Printable. As the library author I prefer to keep the implementation details in the .cpp file. I realize you are copying the format of IPAddress. I also have some questions. I'm a C guy learning c++, so forgive my ignorance.

Should this be return &_mac.bytes[index];? I realize this is the same implementation as IPAddress.h.

    uint8_t& operator[](int index)
    {
        return _mac.bytes[index];
    }

What do these do? Are they needed for PrintTo()? It looks like they return a pointer to the class's private variables, which I consider extremely dangerous.

    operator const uint8_t*() const
    {
        return _mac.bytes;
    }
    operator const uint64_t*() const
    {
        return &_mac.val;
    }

[SuGlider]

Should this be return &_mac.bytes[index];? I realize this is the same implementation as IPAddress.h.

    uint8_t& operator[](int index)
    {
        return _mac.bytes[index];
    }

uint8_t operator[](int index) const is used in getting operation, when there is no change to the memory, just reading values, for instance,
int8_t a1 = addr[1];

But, just the operator[] can't resolve something like addr[2] = 255;
For that, it's necessary a setting operator, uint8_t& operator[](int index), which returns the reference to the element in the array.

int8_t& is a reference to int8_t, in other words, it returns int8_t* type.

What do these do? Are they needed for PrintTo()? It looks like they return a pointer to the class's private variables, which I consider extremely dangerous.

    operator const uint8_t*() const
    {
        return _mac.bytes;
    }
    operator const uint64_t*() const
    {
        return &_mac.val;
    }

Those are for const type pointer casting... Read Only.
It makes easy for any sort of byte-to-byte manipulation, FancyPrint() or any other sort of data processing.

  const uint8_t *ip_8 = MacAddress(0x00, 0xCD, 0x01, 0xFE, 0xB2, 0xF0);
  const uint32_t *ip_64 = MacAddress(ip_8);

[mrengineer7777]

But, just the operator[] can't resolve something like addr[2] = 255; For that, it's necessary a setting operator, uint8_t& operator[](int index), which returns the reference to the element in the array.

Ah, that's helpful.

Those are for const type pointer casting... Read Only. It makes easy for any sort of byte-to-byte manipulation, FancyPrint() or any other sort of data processing.

  const uint8_t *ip_8 = MacAddress(0x00, 0xCD, 0x01, 0xFE, 0xB2, 0xF0);
  const uint32_t *ip_64 = MacAddress(ip_8);

Since it's read only and improves functionality I'll allow it.

Now that I understand the functionality, I'll update my test project to verify the new operators. I'll post a link when I update, likely tomorrow.

@SuGlider I would like to move the implementation details to the .cpp file. Any objections?

[SuGlider]

Please fell free to change anything.

[mrengineer7777]

@SuGlider I need help testing the printTo() function. I don't understand how to use it. I'll post the updates I've done so far while I wait for your reply.

[mrengineer7777]

Updated test project. Does not test printTo() yet. https://github.com/mrengineer7777/MA_Test

[SuGlider]

@SuGlider I need help testing the printTo() function. I don't understand how to use it. I'll post the updates I've done so far while I wait for your reply.

Printable is an Interface to a Print object:
size_t MacAddress::printTo(Print& p)

The common Classes that inherit from Print Class in ESP Arduino are:

• HardwareSerial, USBCDC, HWCDC
• BluetoothSerial, plus some USB based classes
• File, EEPROM, F_Fat, LittleFSFS, SDMMCFS, SPIFSFS, SDFS
• I2SClass, TwoWire -- For some reason SPIClass isn't part of this list...
• WiFiClient, WiFiServer, AsyncUDP, AsyncUDPPAcket, AsyncUDPMessage, UDP, WiFiUDP
• StreamString

  Serial.begin(115200);
  MacAddress addr(0x00, 0xCD, 0x01, 0xFE, 0xB2, 0xF0);

  // it will print the mac address to Serial 
  addr.printTo(Serial);
  Serial.println();           // the line above doesn't send \n

  // it will print the mac address to a String
  StreamString sstr;
  addr.printTo(sstr);
  Serial.println(sstr);
  

[mrengineer7777]

Passes test project. Ready for review.

[me-no-dev]

@mrengineer7777 now it's time to finally review properly and merge your PR.
Here are some remarks:

• Have a look at the official IPAddress class from Arduino Core API: https://github.com/arduino/ArduinoCore-API/blob/master/api/IPAddress.h
• bool MacAddress::fromCStr(const char *buf); should become bool MacAddress::fromString(const char *buf); and should also allow a new constructor MacAddress::MacAddress(const char *macstr);
• int MacAddress::toCStr(char *buf); should become int MacAddress::toString(char *buf);
• The rest looks OK, but we should check int MacAddress:: fromString(const char *buf); for possible pitfalls
• Also please remove the friend classes for now

[me-no-dev]

MacAddress8 should probably be supported in MacAddress, just like IPv6 is supported in IPAddress. That will make things much cleaner and robust

[mrengineer7777]

@me-no-dev I agree on combining MacAddress into one file. Will take some time for me to refactor this.

[me-no-dev]

Sure thing @mrengineer7777 :) Better be done once correctly, than having to change it later or even worst, having to live with it for the rest of your life 😆

[VojtechBartoska]

any progress on this @mrengineer7777? Thanks a lot in advance for your contribution.

[SuGlider]

@mrengineer7777 - it sounds like the branch of this PR doesn't allow us to change the code / rebase it.

[SuGlider]

We may consider it for 3.0.0 instead of 3.0.0-RC1.

[P-R-O-C-H-Y]

Closing this PR in favour of #9304. Thanks @mrengineer7777