search

etherkit / Si5351Arduino

Library for the Si5351 clock generator IC in the Arduino environment
GNU General Public License v3.0
233 stars 95 forks source link

Library writes to EEPROM. #1

Closed la3pna closed 9 years ago

la3pna commented 9 years ago

It may be bad practice to have the library write directly to an (undisclosed) address in the EEPROM.

si5351.set_correction(-900); should apply the factor, but the constant should be read from the eeprrom when the correction is set, making this string to be: si5351.set_correction(EEPROM.read(address)); By this, the eeprom value can be changed, by actual usage of the string si5351.set_correction(-900); no information should be written to the eeprom.

This requires a bit more formal coding when initiating the library, but then the user have control over the address range in the eeprom.

I know this is a minor issue, comments to this appreaciated.

NT7S commented 9 years ago

Yes, you are correct about this. I'll push this change soon, along with some code to set the 7-bit phase register.

oe1wkl commented 9 years ago

I would endorse this - I need to store other things for my application in EEPROM. Right now, I am afraid that there will be a clash (and, I am using a different EEPROM library.... whatever side affects that might have...) Any chance that this change will be made soon?

NT7S commented 9 years ago

Yes, I'll be removing the EEPROM code from the dev branch and merging dev in to master in the near future.

la3pna commented 9 years ago

Another benefit of removing the eeprom code is that the library will work on the DUE, Tensy and other Non-AVR boards.

2015-03-31 16:51 GMT+02:00 Jason Milldrum notifications@github.com:

Yes, I'll be removing the EEPROM code from the dev branch and merging dev in to master in the near future.

— Reply to this email directly or view it on GitHub https://github.com/etherkit/Si5351Arduino/issues/1#issuecomment-88117875 .

Please avoid sending me Word or PowerPoint attachments. See http://www.gnu.org/philosophy/no-word-attachments.html PDF is an better alternative and there are always LaTeX!

NT7S commented 9 years ago

EEPROM code removed in latest commit, should be more portable across platforms.