Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Hardcode EEPROM size #62

Closed
fg89o opened this issue Jul 12, 2020 · 6 comments
Closed

Hardcode EEPROM size #62

fg89o opened this issue Jul 12, 2020 · 6 comments
Assignees
Labels
bug Something isn't working enhancement New feature or request

Comments

@fg89o
Copy link

fg89o commented Jul 12, 2020

I think there should be a posibility to set EEPROM size in begin() method. Before your last merged I did a test with an EEPROM_offset and works correctly but changing the line

EEPROM.begin(512).

For example, If I need 400 bytes for my main project and I want another 512 bytes for INA library the begin method should set EEPROM to 912. Right?. You can expose antoher variable to set EEPROM size and initializate to a default value to have backwards compatibility.

In the other hand, you calculate max_devices including the EEPROM_offset variable ,If the library is not allowed to use the offset, the maximum devices should be:

maxDevices = (EEPROM_size - _EEPROM_offset ) / sizeof(inaEE);

instead of

maxDevices = (_EEPROM_offset + EEPROM_size) / sizeof(inaEE);

I did all the tests with an ESP32. Hope something of that help you to improve the library,

@fg89o fg89o added the bug Something isn't working label Jul 12, 2020
SV-Zanshin pushed a commit that referenced this issue Jul 12, 2020
@SV-Zanshin SV-Zanshin mentioned this issue Jul 12, 2020
13 tasks
@SV-Zanshin
Copy link
Collaborator

👍 I missed that, good catch and I've added the correct offset math to the code.

@fg89o
Copy link
Author

fg89o commented Jul 12, 2020

Are you going to create the EEPROM size variable too?.

@SV-Zanshin
Copy link
Collaborator

I wanted to fix the actual bug first, before the new release was published.

I still need to think about changing the begin() method, which is why this issue is still open.

@SV-Zanshin
Copy link
Collaborator

What I will do is add another public uint16_t variable "_EEPROM_size" which is only valid for the ESP32 and ESP8266 platform. It defaults to 512 but can be changed before the begin() is called.

@SV-Zanshin SV-Zanshin added the enhancement New feature or request label Jul 12, 2020
SV-Zanshin pushed a commit that referenced this issue Jul 12, 2020
Added new public variable "_EEPROM_size" for ESP32 and ESP8266 which defaults to 512 bytes but allows changing the size prior to the begin() call.
@fg89o
Copy link
Author

fg89o commented Jul 12, 2020

Perfect! Thank you for your support.

@SV-Zanshin
Copy link
Collaborator

OK, that should work!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants