Skip to content

place mac to SRAM instead of flash#397

Closed
nagimov wants to merge 1 commit intonjh:masterfrom
nagimov:patch-1
Closed

place mac to SRAM instead of flash#397
nagimov wants to merge 1 commit intonjh:masterfrom
nagimov:patch-1

Conversation

@nagimov
Copy link

@nagimov nagimov commented Aug 12, 2020

This example doesn't work when mac is placed to flash (using nano v3 with old bootloader)

This example doesn't work when mac is placed to flash (using nano v3 with old bootloader)
Copy link
Contributor

@nuno-silva nuno-silva left a comment

Choose a reason for hiding this comment

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

Indeed, ether.begin seems to assume macaddr is in RAM:

uint8_t EtherCard::begin (const uint16_t size,
const uint8_t* macaddr,
uint8_t csPin) {
using_dhcp = false;
#if ETHERCARD_STASH
Stash::initMap();
#endif
copyMac(mymac, macaddr);

void EtherCard::copyMac (uint8_t *dst, const uint8_t *src) {
memcpy(dst, src, ETH_LEN);
}

Perhaps it would be best to overload ether.begin with support for passing a MAC in PROGMEM, but at least this makes it work for now...

@nagimov
Copy link
Author

nagimov commented Aug 13, 2020

@nuno-silva
Yes if it could be implemented similarly to dnsLookup and accept optional fromRam flag that would be ideal:

bool EtherCard::dnsLookup (const char* name, bool fromRam) {

@nuno-silva
Copy link
Contributor

No need for an additional argument :)
It just needs to be overloaded to receive const __FlashStringHelper* instead of const uint8_t*, like this:

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

The overloaded begin method would just need to read the uint8_t array from flash to a temporary buffer in the stack/RAM and call the regular const uint8_t* method. It's also possible to avoid the temporary array by "copy pasting" the regular method and changing the memcpy call to a loop reading from flash :)

@nuno-silva
Copy link
Contributor

I could implemented it, but I have no setup to test it at the moment.

@nagimov
Copy link
Author

nagimov commented Aug 14, 2020

Better solution is in #398

@nagimov nagimov closed this Aug 14, 2020
nuno-silva added a commit to nuno-silva/ethercard that referenced this pull request Aug 16, 2020
nuno-silva added a commit to nuno-silva/ethercard that referenced this pull request Nov 22, 2020
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