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

Adding methods to handle reading and writing of the EEPROM through li… #87

Closed
wants to merge 2 commits into from

Conversation

sgoadhouse
Copy link
Contributor

…busb ctrl_transfer(). I have tested these quite a bit with a FT4232H but have not tested with older FTDI devices. If these are accepted, it may be good to duplicate the write eeprom warning in my code changes to the README.

@eblot
Copy link
Owner

eblot commented Feb 20, 2018

I will give it a try with a real device, but I think it would be nice to add this feature to PyFtdi.

There are a couple of things to fix (move doc/URL to .rst files, remove extra lines and deal with wide lines), but I can take care of this.

I'm worrying a bit with the address management: I think it is safer to raise an exception if the client attempts to access an invalid address than silently ignore the error and access an unexpected location - especially if this may corrupt the EEPROM contents and eventually brick the FTDI :-)

Thanks.

@sgoadhouse
Copy link
Contributor Author

Sorry about the formatting issues. I am still getting use to the format. Thanks for taking care of them.

Yes, you are absolutely correct that an exception should be raised, at least in _write_eeprom_raw() and _write_eeprom_word(). For the “public” method write_eeprom(), since it reads the entire contents of the EEPROM and inserts the new data as bytes, addr and data length can safely be odd. I think that it makes for a more natural interface to keep them as a byte address and byte data. So an exception for odd addr or odd len(data) is not required in write_eeprom(). For example, if addr is odd, addr is decremented by 1 for writing, but it is simply writing back the byte that was read from the EEPROM.

@eblot
Copy link
Owner

eblot commented Feb 22, 2018

Sorry sgoadhouse, I'm really busy by now - I'll try to review all your changes when I get some spare time; thanks for the fixes.

@sgoadhouse
Copy link
Contributor Author

sgoadhouse commented Feb 22, 2018 via email

@eblot
Copy link
Owner

eblot commented Feb 26, 2018

Checking at the pull request changes, I do not really see where a read-modify-write is performed. As there is no "erase" feature, I do not get why extending a non-aligned write with '0xFF' is a better value that any random value (vs. accessing a NOR or NAND flash, where '0xFF' would make sense). Did I miss something?

Moreover, if addr & 1 && length & 1, we end up reading out two extra bytes, for example, which get not truncated (if the return buffer is 2 byte longer than requested). Sounds weird, does not it?

Nevertheless I've imported your code and started to work on it, see eeprom-access branch (WIP)

@sgoadhouse
Copy link
Contributor Author

sgoadhouse commented Feb 27, 2018 via email

@eblot
Copy link
Owner

eblot commented Feb 27, 2018

I've added those sanity checks in the latest dev branch (eeprom-access), and the code should now deal with any kind of access - 16 bit aligned or not. I've added a couple of unit test for this feature. I have not tried the write feature - only simulated it for now.

The very useful think now would be to add a new file/class with helper function to encode/decode the EEPROM content, but that's quite a piece of code.

@sgoadhouse
Copy link
Contributor Author

sgoadhouse commented Feb 27, 2018 via email

@eblot
Copy link
Owner

eblot commented Feb 28, 2018

Yes, I know about the stupid FTDIchip NDA policy. Back in the inception days of PyFTDI, I had to reverse engineer the Windows USB driver communication to support FT4232 devices... :-( They are still many unknown areas with FTDI devices, such as timings for example.

I already had a look @ libftdi, and adding a comprehensive EEPROM encode/decode support will require several hours of (boring) work :-)

I think at least common EEPROM needs (such as changing the serial number and the product description string) could be easily added. I just do not have spare time for now to write this feature...

@eblot
Copy link
Owner

eblot commented Nov 21, 2019

Ported and adapted to a193973, thanks

@eblot eblot closed this Nov 21, 2019
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