nonNoise / PyMCP2221A

MCP2221 & MCP2221A work in Python.
MIT License
36 stars 21 forks source link

VID/PID unused #4

Closed aaretali closed 5 years ago

aaretali commented 5 years ago

VID/PID are provided to the constructor and default values are provided, but constructor doesn't even attempt to use them, using hard-coded values same as defaults instead.

nonNoise commented 5 years ago

PyMCP2221A.py

class PyMCP2221A:
    def __init__(self,VID = 0x04D8,PID = 0x00DD,devnum = 0):
        self.mcp2221a = hid.device()
        self.mcp2221a.open_path(hid.enumerate(0x04D8, 0x00DD)[devnum]["path"])

Oh... Sorry I'm mistake. Do you want to change VID? Do you want to distinguish and use the same VID device? It will be changed in the next version.

aaretali commented 5 years ago

The right thing would be to have:

self.mcp2221a.open_path(hid.enumerate(VID, PID)[devnum]["path"])

This way, if someone wants to change VID/PID, they can. Defaults are fine for most cases. The library worked for me without changes, this is just an issue of correctness, 'to do what is expected'.

vrbadev commented 5 years ago

Could you please correct the mistake? I use custom VID and PID and with this error I can't rely on this library.

aaretali commented 5 years ago

The fix is in the message thread above, so just download the package and fix that one line. It will be changed in next version anyway, so once you upgrade, you'll have a workable version.

nonNoise commented 5 years ago

OK No problem. I released a fix. If you can confirm, please.

v1.4.0 pip3 install -U PyMCP2221A

aaretali commented 5 years ago

Confirmed, fixed.