Support various CST816S touch controller chip IDs#522
Support various CST816S touch controller chip IDs#522dudasdavid wants to merge 2 commits intolvgl-micropython:mainfrom
Conversation
kdschlosser
left a comment
kdschlosser
left a comment
There was a problem hiding this comment.
1 suggestion and asking what you think about it.
| if chip_id not in (_ChipIDValue, _ChipIDValue2): | ||
| raise RuntimeError(f'Incorrect chip id ({hex(_ChipIDValue)})') | ||
| if chip_id not in _ExpectedChipIDs: | ||
| raise RuntimeError(f'Incorrect chip id: {hex(chip_id)}, expected: {", ".join(hex(v) for v in _ExpectedChipIDs)}') |
There was a problem hiding this comment.
Instead of checking _ExpectedChipIDs directly set it as the default for a parameter of the constructor called valid_ids and check to see if the ID is in that list. This makes it possible for the user to pass an id into the driver if it happens to not be already added.
right now the constructor looks like this...
def __init__(
self,
device,
reset_pin=None,
touch_cal=None,
startup_rotation=pointer_framework.lv.DISPLAY_ROTATION._0, # NOQA
debug=False
):
self._tx_buf = bytearray(2)
self._tx_mv = memoryview(self._tx_buf)
self._rx_buf = bytearray(1)
self._rx_mv = memoryview(self._rx_buf)
self._device = device
if not isinstance(reset_pin, int):
self._reset_pin = reset_pin
else:
self._reset_pin = machine.Pin(reset_pin, machine.Pin.OUT)
if self._reset_pin:
self._reset_pin.value(1)
self.hw_reset()
self.auto_sleep = False
self._read_reg(_ChipID)
print('Chip ID:', hex(self._rx_buf[0]))
chip_id = self._rx_buf[0]
self._read_reg(_RegisterVersion)
print('Touch version:', self._rx_buf[0])
self._read_reg(_ProjID)
print('Proj ID:', hex(self._rx_buf[0]))
self._read_reg(_FwVersion)
print('FW Version:', hex(self._rx_buf[0]))
if chip_id not in (_ChipIDValue, _ChipIDValue2):
raise RuntimeError(f'Incorrect chip id ({hex(_ChipIDValue)})')with your changes it looks like this...
def __init__(
self,
device,
reset_pin=None,
touch_cal=None,
startup_rotation=pointer_framework.lv.DISPLAY_ROTATION._0, # NOQA
debug=False
):
self._tx_buf = bytearray(2)
self._tx_mv = memoryview(self._tx_buf)
self._rx_buf = bytearray(1)
self._rx_mv = memoryview(self._rx_buf)
self._device = device
if not isinstance(reset_pin, int):
self._reset_pin = reset_pin
else:
self._reset_pin = machine.Pin(reset_pin, machine.Pin.OUT)
if self._reset_pin:
self._reset_pin.value(1)
self.hw_reset()
self.auto_sleep = False
self._read_reg(_ChipID)
print('Chip ID:', hex(self._rx_buf[0]))
chip_id = self._rx_buf[0]
self._read_reg(_RegisterVersion)
print('Touch version:', self._rx_buf[0])
self._read_reg(_ProjID)
print('Proj ID:', hex(self._rx_buf[0]))
self._read_reg(_FwVersion)
print('FW Version:', hex(self._rx_buf[0]))
if chip_id not in _ExpectedChipIDs:
raise RuntimeError(f'Incorrect chip id: {hex(chip_id)}, expected: {", ".join(hex(v) for v in _ExpectedChipIDs)}')and I think the code block below would be ideal.
def __init__(
self,
device,
reset_pin=None,
touch_cal=None,
startup_rotation=pointer_framework.lv.DISPLAY_ROTATION._0, # NOQA
debug=False,
valid_ids=_ExpectedChipIDs
):
self._tx_buf = bytearray(2)
self._tx_mv = memoryview(self._tx_buf)
self._rx_buf = bytearray(1)
self._rx_mv = memoryview(self._rx_buf)
self._device = device
if not isinstance(reset_pin, int):
self._reset_pin = reset_pin
else:
self._reset_pin = machine.Pin(reset_pin, machine.Pin.OUT)
if self._reset_pin:
self._reset_pin.value(1)
self.hw_reset()
self.auto_sleep = False
self._read_reg(_ChipID)
print('Chip ID:', hex(self._rx_buf[0]))
chip_id = self._rx_buf[0]
self._read_reg(_RegisterVersion)
print('Touch version:', self._rx_buf[0])
self._read_reg(_ProjID)
print('Proj ID:', hex(self._rx_buf[0]))
self._read_reg(_FwVersion)
print('FW Version:', hex(self._rx_buf[0]))
if chip_id not in valid_ids:
raise RuntimeError(f'Incorrect chip id: {hex(chip_id)}, expected: {", ".join(hex(v) for v in valid_ids)}')This would allow users to pass their chip id into the constructor if there are any additional ones that are not already added.
what are your thoughts?
There was a problem hiding this comment.
I'm only wondering how user friendly it is that the user need to pass a list instead of his exact chip ID? Shouldn't we rather allow the user to pass a single chip which is then appened to the expected list? Or being flexible with both options with some extra code in the constructor - although I'm not a big fan of making it bloated.
There was a problem hiding this comment.
you could do an instance check to see if it is an int and then do a direct comparison. it's not that much additional code. I would default it to being the tuple and do the instance check..
your thoughts?
…n passed arguments int and tuple are accepted
|
Sorry for late reply, what do you think now? |
| if chip_id not in valid_ids: | ||
| raise RuntimeError(f'Incorrect chip id: {hex(chip_id)}, expected: {", ".join(hex(v) for v in valid_ids)}') | ||
| else: | ||
| raise RuntimeError(f'Expected chip id needs to be int or tuple') |
There was a problem hiding this comment.
if isinstance(valid_ids, (list, tuple)):
if chip_id not in valid_ids:
raise RuntimeError(f'Incorrect chip id: {hex(chip_id)}, expected: {", ".join(hex(v) for v in valid_ids)}')
elif chip_id != valid_ids:
raise RuntimeError(f'Incorrect chip id: {hex(chip_id)}, expected: {hex(valid_ids)}')That should be all that needs to be added.
2 participants
I have a development board with CST816S with 0xb4 chip ID, I added support for 3 chip IDs now and a more flexible extension for the future.