Skip to content

EtherCard::begin(): support reading MAC address from PROGMEM#399

Closed
nuno-silva wants to merge 1 commit intonjh:masterfrom
nuno-silva:mac-progmem
Closed

EtherCard::begin(): support reading MAC address from PROGMEM#399
nuno-silva wants to merge 1 commit intonjh:masterfrom
nuno-silva:mac-progmem

Conversation

@nuno-silva
Copy link
Contributor

fixes #397 (ntpClient example)
closes #398

@nuno-silva
Copy link
Contributor Author

@nagimov please test :)

Make sure at least the ntpClient example runs correctly, as well as some other random example that does not place the MAC in PROGMEM.

@nagimov
Copy link

nagimov commented Aug 17, 2020

Tested example https://github.com/njh/EtherCard/blob/master/examples/ntpClient/ntpClient.ino with ntp server changed to pool.ntp.org

When mymac is assigned without PROGMEM everything works, dhcp gets ip, ntp syncs ok, reported mac matches the one specified.

When mymac is assigned with PROGMEM everything works as well, however reported mac (observed at the router side) in this case is always 4E:43:32:38:6A:36, regardless from what's in mymac (tested using a bunch of random macs, 00:00:00:00:00:00 and FF:FF:FF:FF:FF:FF).

@nagimov
Copy link

nagimov commented Aug 17, 2020

I think I found the issue. I tried implementing it in a similar way before falling back to optional fromRam parameter. The problem is that compiler doesn't seem to "know" the difference between const uint8_t* macaddr and const __FlashStringHelper *macaddr. Debugging using Serial.print() shows that regardless of whether I specify PROGMEM for mymac or not, functions with const uint8_t* macaddr are called instead of const __FlashStringHelper *macaddr. This causes "wrong" copyMac to call memcpy() instead of memcpy_P().

Not sure if that's a bug or intended behavior of avr compiler.

My board setup:

$ arduino-cli board details arduino:avr:nano
Board name:             Arduino Nano                                                            
Board fqbn:             arduino:avr:nano                                                        
Board propertiesId:     nano                                                                    
Board version:          1.8.3                                                                   

Official Arduino board: ✔                                                                       

Package name:           arduino                                                                 
Package maintainer:     Arduino                                                                 
Package URL:            https://downloads.arduino.cc/packages/package_index.json                
Package websiteURL:     http://www.arduino.cc/                                                  
Package online help:    http://www.arduino.cc/en/Reference/HomePage                             

Platform name:          Arduino AVR Boards                                                      
Platform category:      Arduino                                                                 
Platform architecture:  avr                                                                     
Platform URL:           http://downloads.arduino.cc/cores/avr-1.8.3.tar.bz2                     
Platform file name:     avr-1.8.3.tar.bz2                                                       
Platform size (bytes):  4941548                                                                 
Platform checksum:      SHA-256:de8a9b982477762d3d3e52fc2b682cdd8ff194dc3f1d46f4debdea6a01b33c14

Required tools:         arduino:avr-gcc                                                            7.3.0-atmel3.6.1-arduino7

Required tools:         arduino:avrdude                                                            6.3.0-arduino17          

Required tools:         arduino:arduinoOTA                                                         1.3.0                    

Option:                 Processor                                                                  cpu                      
                        ATmega328P                                                               ✔ cpu=atmega328            
                        ATmega328P (Old Bootloader)                                                cpu=atmega328old         
                        ATmega168                                                                  cpu=atmega168

@nuno-silva
Copy link
Contributor Author

@nagimov thanks for testing. I'll have to investigate it further :/

@nuno-silva nuno-silva marked this pull request as draft August 18, 2020 19:18
@nuno-silva
Copy link
Contributor Author

The PROGMEM is just an attribute and has no effect when overloading functions :/

The __FlashStringHelper, however, is a class:

https://github.com/arduino/ArduinoCore-avr/blob/60f0d0b125e06dbf57b800192c80e5f60d681438/cores/arduino/WString.h#L37-L38

hence why it works when overloading the String constructor, the print method, etc:

https://github.com/arduino/ArduinoCore-avr/blob/3055c1efa3c6980c864f661e6c8cc5d5ac773af4/cores/arduino/Print.cpp#L44-L55

However, since this depends on the __FlashStringHelper class to bind the call to the correct method, the PROGMEM attribute has no effect in this regard.
The overloading only works with the F macro, because it casts the pointer to __FlashStringHelper*.

Therefore, I'll re-implement this using the fromRam parameter :/

nuno-silva added a commit to nuno-silva/ethercard that referenced this pull request Nov 22, 2020
Using an optional fromRam argument instead of overloading (see njh#399).
Defaults to fromRam=true so as not to break existing code.

closes njh#399
closes njh#398
@nuno-silva nuno-silva closed this Jul 15, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants