DancingQuanta / pyusbiss

Python API for USB-ISS Multifunction USB Communications Module
MIT License
2 stars 3 forks source link

USBISS #4

Closed gwdehaan closed 6 years ago

gwdehaan commented 6 years ago

Description

Testing the usbiss base class and ISP protocol class gave some runtime errors, as the class properties where defined with the self. prefix.

What I Did

  1. correct the typos in usbiss and spi class.

  2. in the 'old' module versioninfo was shown by default, you now added the _repr() method, but it is never called within the module, so I was wondering how you intended to use this.

for now I added it to the USBISS.init()

    def __init__(self, port):

        # Open serial port
        serial_opts = {"port": port,
                       "baudrate": 9600,
                       "parity": serial.PARITY_NONE,
                       "bytesize": serial.EIGHTBITS,
                       "stopbits": serial.STOPBITS_ONE,
                       "xonxoff": False,
                       "timeout": 1}
        self.serial = serial.Serial(**serial_opts)
        self.get_iss_info()
        self.get_iss_serial_no()
        print(self.__repr__())

with these changes I succesfully ran the following test. As I don't have a spi device at this point in time I can only confirm that spi initialisation works

from spi import SPI

t=SPI('COM3', spi_mode = 1, freq = 25000)

Output :

36-32/python.exe c:\Users\Geert\Documents\Project\pyusbiss\usbiss\spi-test.py
The module ID is 7
The firmware version is 0x4
The current operating mode is 0x91
The serial number is b'00006910'

Edits : typos

DancingQuanta commented 6 years ago

I know that in the old version that information about USB-ISS is printed out. However this may be undesirable and a waste of few clock cycles and I do not know of any python library that automatically prints out upon an event unless asked to do so.

I rather prefer that the successful connection to be silent. Normally, a failed connection will scream out something. So __repr__() method allow a user to inspect the opened connection after a successful connection.

I am going to push out some other changes in USB-ISS which is more abstraction. For example, you get three new methods that: write bytearray to USB-ISS, read bytes from USB-ISS and decode bytes. This will make SPI and other classes more simpler. Do backup your changes by

git checkout -b old_classes

before pulling.

gwdehaan commented 6 years ago
def decode(self, data):
        decoded = []
        for i in range(0, len(data)):
            unpacked = struct.unpack('B', data[i + 1: i + 2])[0]
            decoded = decoded + [unpacked]
        return decoded

This method gives an error

struct.error: unpack requires a byte object of length 1

I think the slicing is 1 position off. When feeding 0x7, 0x4, 0x40, decoded wil stop with the error at [4,64]

I'm a bit confused as it seems a correct rewrite of this fragment from the original xfer method.

decoded = [struct.unpack('B', response[i + 1: i + 2])[0] for i in range(0, len(data))]

DancingQuanta commented 6 years ago

Ah yes, that function is not correct. The data received is actually 1+len(data). The first byte is ACK. I believe that the first byte should be stripped off and the correction to the line

unpacked = struct.unpack('B', data[i + 1: i + 2])[0]

would be

unpacked = struct.unpack('B', data[i: i + 1])[0]

The original code assume that the number of bytes send out plus SPI_CMD byte is equal to the number of bytes plus ACK byte. See that response is not the same as data in terms of length. So now the decode should only be fed with actual data without any USB-ISS specific bytes. I will push out a commit tonight. The reason why I rewrote this code is to break it down and make sure each line is under 80 characters.

Please note that the functions get_iss_* do not receive ACK bytes. I think you should read all of USB-ISS documentation and determine if ACK/NACK is universial or if there are exception to this rule (apart from git_iss_*). If ACK is universial then perhaps I will modify iss_read to inspect the first byte.

DancingQuanta commented 6 years ago

Bought my own USB-ISS and did a few SPI loopback tests. I have refined the SPI class to get things to work.

gwdehaan commented 6 years ago

Tested the new spi and initialization works now, I was still getting my head around the new decorator style that you introduced with the latest version.

I installed usbiss with python setup.py install and always got an error on from . import USBISS. My workaround was to use from usbiss import USBISS but perhaps you can share your intended use of the USBISS module.

Now that you have your own USB-ISS how do you want to proceed ?

DancingQuanta commented 6 years ago

Please use setup.py in top-level directory, eg outside of usbiss.

I am going to look into writing tests and uploading to pypi to get things going.

Will you be working on IO and I2C?

I suggest familiarising yourself with pyftdi. It is a very complex driver but uses a number of protocols like USB-ISS. Happily USB-ISS is simpler :). I am looking for some inspiration from this driver, especially tests.

DancingQuanta commented 6 years ago

I am starting to feel that inheriting USBISS class in other protocol classes don't cut it. Each protocol classes will have a number of methods and mixing them with the parent class means that we are restricted in naming them.

I have read through pyftdi's source code, and found that the Ftdi controlling class is not inherited by protocol classes but are used as an interface.

class SpiController();

  def __init__():
    self._ftdi = Ftdi()

This makes more sense to me. I will make a commit and create anotther branch classes2.