search

eblot / pyftdi

FTDI device driver written in pure Python
Other
509 stars 212 forks source link

Additional EEPROM mock tests to cover regression #304

Closed len0rd closed 1 year ago

len0rd commented 2 years ago

Verified the new tests fail without https://github.com/eblot/pyftdi/commit/7cdfcd544a08a605065db0f5bf274947187f44c0 and pass once its applied. Added more tests to verify disabling mirroring on mirror-capable devices results in a non-mirrored eeprom. Verify devices that cant mirror (230x/232r) dont.

MarcFinetRtone commented 1 year ago

Hello @eblot : could it be possible to merge this (and the eeprom_regression branch to main (and issue a new release)) ? I stumbled upon the Error: EEPROM does not support mirroring error while trying to update a serial for a FT232RL.

eblot commented 1 year ago

Hi @MarcFinetRtone

I did not get a chance to try it on a real FT232RL device. Can you confirm the patch works on a real device, and I'll merge it. Thanks.

tuna-f1sh commented 1 year ago

I still have an issue attempting to write the EEPROM on a FT230X: Error: EEPROM does not support mirroring.

It's because lines https://github.com/eblot/pyftdi/blob/master/pyftdi/eeprom.py#L693 and https://github.com/eblot/pyftdi/blob/master/pyftdi/eeprom.py#L695 call the self.mirror_sector property without a check for is_mirroring_enabled. Fixing this though, I get that:

  File "/Users/john/git/pyftdi/pyftdi/eeprom.py", line 711, in _generate_var_strings                                                                            │
    self._eeprom[tbl_pos] = data_pos                                                                                                                            │
    ~~~~~~~~~~~~^^^^^^^^^                                                                                                                                       │
ValueError: byte must be in range(0, 256)

Quickly checked memory mapping and it seems correct so not sure what's happening here. Any ideas? Is a bytearray index limited to a byte?!

tuna-f1sh commented 1 year ago

Ok with more thought I realised that this was actually due to data_pos being > byte. I reduced the length of the strings I was trying to set and that fixed it. Would be nice to raise a more verbose exception for this however, the commit mentioning this fixes this issue though.

MarcFinetRtone commented 1 year ago

Hi @MarcFinetRtone

I did not get a chance to try it on a real FT232RL device. Can you confirm the patch works on a real device, and I'll merge it. Thanks.

Sorry I missed the notification. Yes, I confirm that the https://github.com/eblot/pyftdi/tree/dev/ebl/eeprom_regression branch fixes the issue on a real FT232RL. However I did not test this commit (regarding tests), but it seems that's the only PR for EEPROM regression issue.

eblot commented 1 year ago

Delivered to main as 0.55.1. Thanks.