masterx1981 commented 3 years ago

Description

Added the function for read the UUID store on a 6byte EEPROM (74901) or 8byte EEPROM (74902). On other devices not return anything. function name : readEEPROM

Type of change

Please delete options that are not relevant.

[ ] Bug fix (non-breaking change which fixes an issue)
[x ] New feature (non-breaking change which adds functionality)
[ ] Breaking change (fix or feature that would cause existing functionality to not work as expected)
[ ] This change requires a documentation update

How Has This Been Tested?

Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration

Tested against an MCP74902, reading correctly all the 8 bytes.

Test Configuration:

Arduino version: Industruino D21G, Arduino Zero compatible
Arduino Hardware:
SDK: (Arduino IDE)
Development system: (Windows)

Checklist:

[ ] The changes made link back to an existing issue number
[ ] I have performed a self-review of my own code
[x] My code follows the style guidelines of this project
[x] I have commented my code, particularly in hard-to-understand areas
[ ] The code adheres to the Google Style Guide
[ ] The code follows the existing program documentation style using doxygen
[ ] I have made corresponding changes to the documentation / Wiki Page(s)
[x] My changes generate no new warnings
[x] The automated TRAVIS-CI run has a status of "passed"
[x] I have checked potential areas where regression errors could occur and have found no issues
[x] I have added tests that prove my fix is effective or that my feature works
[x] New and existing unit tests pass locally with my changes

SV-Zanshin commented 3 years ago

I didn't know that these new devices existed. MDP79400/MCP79401 and MCP79402. Are they truly backwards compatible with the MCP7940? If so, I could add in this pull request and also update the documentation as well as the library.properties to reflect this addition.

masterx1981 commented 3 years ago

Hi! Yes, i have a device with a 74902 (8byte eeprom), they work perfectly with this library. Seem all backwards compatible. I use the deviceStatus, deviceStart, getBattery, setBattery, now, getPowerFail, getPowerDown (that i discovered now not return the whole date, only time), need to check if it's a bug), getPowerUp (same as powerdown), writeRAM, readRAM, and calibrateOrAdjust, and excluding those 2 things on power fail all works perfectly. I've not implemented the writeeeprom as i not find useful 8byte of eeprom, they are great for example as mac address, or as UUID for a device. Thanks for your work on this library!!

SV-Zanshin commented 3 years ago

I’ll see about ordering one online and testing it!

The “getPowerDown()” issue isn’t a bug – that is the way the device stores the data.

-Arnd.

From: masterx1981 notifications@github.com Sent: 01 January 2021 17:32 To: Zanduino/MCP7940 MCP7940@noreply.github.com Cc: Arnd zanshin_github@sv-zanshin.com; Assign assign@noreply.github.com Subject: Re: [Zanduino/MCP7940] Update MCP7940.h (#58)

Hi! Yes, i have a device with a 74902 (8byte eeprom), they work perfectly with this library. Seem all backwards compatible. I use the deviceStatus, deviceStart, getBattery, setBattery, now, getPowerFail, getPowerDown (that i discovered now not return the whole date, only time), need to check if it's a bug), getPowerUp (same as powerdown), writeRAM, readRAM, and calibrateOrAdjust, and excluding those 2 things on power fail all works perfectly. I've not implemented the writeeeprom as i not find useful 8byte of eeprom, they are great for example as mac address, or as UUID for a device. Thanks for your work on this library!!

— You are receiving this because you were assigned. Reply to this email directly, view it on GitHub https://github.com/Zanduino/MCP7940/pull/58#issuecomment-753339624 , or unsubscribe https://github.com/notifications/unsubscribe-auth/AFT5DRCJ7WM4K7FMARW3NJDSXX2JXANCNFSM4VQJYVTQ . https://github.com/notifications/beacon/AFT5DRGP6O7FTUBG4CXJQWLSXX2JXA5CNFSM4VQJYVT2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOFTTQZ2A.gif

masterx1981 commented 3 years ago

Yeah, i've read the pdf of the device ad it not store the year! Unfortunatelly we are on day 1 / month 1 of the year and i've tought that also day/month wasn't retrived, sorry!

SV-Zanshin commented 3 years ago

I'd like to solve this a different way, leaving the original template functions untouched. I've made some changes to the MCP7940.h file and there is now a public function that you can call, "readEUI(,structure)"

uint8_t eui[0];
uint8_t bytes_read = MCP7940.readEUI(0,eui);

Can you give that a try? If it works, then I'll remove this pull request and also update the documentation to reflect the new routine and also that the library works for MCP79400, MCP79401 and MCP79402.

masterx1981 commented 3 years ago

Hi! I've not tried by now as i'm doing some rework of the code. Hope tomorrow to test it. Compiling i get some warnings: C:\Users\User\Documents\Arduino\libraries\MCP7940\src/MCP7940.h: In instantiation of 'uint8_t& MCP7940_Class::readRAM(const uint8_t&, T&) const [with T = DateTime; uint8_t = unsigned char]': C:\Users\User\Documents\Arduino\Industruino\Industruino\Industruino.ino:476:36: required from here C:\Users\User\Documents\Arduino\libraries\MCP7940\src/MCP7940.h:305:13: warning: reference to local variable 'i' returned [-Wreturn-local-addr] uint8_t i = I2C_read((addr % 64) + MCP7940_RAM_ADDRESS, value); ^ C:\Users\User\Documents\Arduino\libraries\MCP7940\src/MCP7940.h: In instantiation of 'uint8_t& MCP7940_Class::readEUI(const uint8_t&, T&) const [with T = unsigned char [8]; uint8_t = unsigned char]': C:\Users\User\Documents\Arduino\Industruino\Industruino\Industruino.ino:497:41: required from here C:\Users\User\Documents\Arduino\libraries\MCP7940\src/MCP7940.h:331:13: warning: reference to local variable 'i' returned [-Wreturn-local-addr] uint8_t i{0}; // return number of bytes read

SV-Zanshin commented 3 years ago

Hello,

I made an error in the return types for those two calls. I’ve corrected it and uploaded the new code to GitHub.

-Arnd.

From: masterx1981 notifications@github.com Sent: 06 January 2021 01:19 To: Zanduino/MCP7940 MCP7940@noreply.github.com Cc: Arnd zanshin_github@sv-zanshin.com; Assign assign@noreply.github.com Subject: Re: [Zanduino/MCP7940] Update MCP7940.h (#58)

Hi! I've not tried by now as i'm doing some rework of the code. Hope tomorrow to test it. Compiling i get dome warnings: C:\Users\User\Documents\Arduino\libraries\MCP7940\src/MCP7940.h: In instantiation of 'uint8_t& MCP7940_Class::readRAM(const uint8_t&, T&) const [with T = DateTime; uint8_t = unsigned char]': C:\Users\User\Documents\Arduino\Industruino\Industruino\Industruino.ino:476:36: required from here C:\Users\User\Documents\Arduino\libraries\MCP7940\src/MCP7940.h:305:13: warning: reference to local variable 'i' returned [-Wreturn-local-addr] uint8_t i = I2C_read((addr % 64) + MCP7940_RAM_ADDRESS, value); ^ C:\Users\User\Documents\Arduino\libraries\MCP7940\src/MCP7940.h: In instantiation of 'uint8_t& MCP7940_Class::readEUI(const uint8_t&, T&) const [with T = unsigned char [8]; uint8_t = unsigned char]': C:\Users\User\Documents\Arduino\Industruino\Industruino\Industruino.ino:497:41: required from here C:\Users\User\Documents\Arduino\libraries\MCP7940\src/MCP7940.h:331:13: warning: reference to local variable 'i' returned [-Wreturn-local-addr] uint8_t i{0}; // return number of bytes read

— You are receiving this because you were assigned. Reply to this email directly, view it on GitHub https://github.com/Zanduino/MCP7940/pull/58#issuecomment-754980493 , or unsubscribe https://github.com/notifications/unsubscribe-auth/AFT5DRHAH4MA4MJVZUC6VCLSYOUABANCNFSM4VQJYVTQ . https://github.com/notifications/beacon/AFT5DRFRROXHYAXB7GIUMKLSYOUABA5CNFSM4VQJYVT2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOFUABNDI.gif

masterx1981 commented 3 years ago

I can confirm that the function outputs correctly the 8 byte eeprom record of my MCP79402. The chip allows also an overwrite of this value, i not know if it's worth to implement a function for overwrite it, 6/8 bytes of storage are useful almost for anything (excluding for an UUID, that is already there, i use it as mac address). Thanks for your work! Your code is very well written, and as new into the c world i've learnt a lot of things reading it.

SV-Zanshin commented 3 years ago

I think I might add a write function as well, that requires some extra code to “unlock” that area. The nice thing about the c++ compiler and the Arduino libraries is that if a function is not used in the program, it will be optimized out and not take any space, so adding these 2 functions won’t increase the library size if they are not used.

-Arnd.

From: masterx1981 notifications@github.com Sent: 06 January 2021 11:19 To: Zanduino/MCP7940 MCP7940@noreply.github.com Cc: Arnd zanshin_github@sv-zanshin.com; Assign assign@noreply.github.com Subject: Re: [Zanduino/MCP7940] Update MCP7940.h (#58)

I can confirm that the function outputs correctly the 8 byte eeprom record of my MCP79402. The chip allows also an overwrite of this value, i not know if it's worth to implement a function for overwrite it, 6/8 bytes of storage are useful almost for anything (excluding for an UUID, that is already there, i use it as mac address). Thanks for your work! Your code is very well written, and as new into the c world i've learnt a lot of things reading it.

— You are receiving this because you were assigned. Reply to this email directly, view it on GitHub https://github.com/Zanduino/MCP7940/pull/58#issuecomment-755213986 , or unsubscribe https://github.com/notifications/unsubscribe-auth/AFT5DRH4LGJDXJQRFMRAJILSYQ2KPANCNFSM4VQJYVTQ . https://github.com/notifications/beacon/AFT5DREJKN53GEJHA7PGF4LSYQ2KPA5CNFSM4VQJYVT2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOFUB2NIQ.gif

SV-Zanshin commented 3 years ago

The requested changes have been implemented as [readEUI()](https://github.com/Zanduino/MCP7940/wiki/readEUI()) and [writeEUI()](https://github.com/Zanduino/MCP7940/wiki/writeEUI()) in a separate development path, making this pull request obsolete.

masterx1981 commented 3 years ago

Thanks for your efforts, i'll test the write function as soon as possible.

Zanduino / MCP7940

Update MCP7940.h #58

Description

Type of change

How Has This Been Tested?

Checklist: