Skip to content

Add Type Annotations #26

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

Merged
merged 3 commits into from
Aug 22, 2022
Merged
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 20 additions & 13 deletions adafruit_tlc5947.py
Original file line number Diff line number Diff line change
Expand Up @@ -74,19 +74,19 @@ class PWMOut:
as it is fixed by the TLC5947 to ~2.4-5.6 mhz.
"""

def __init__(self, tlc5947, channel):
def __init__(self, tlc5947: "TLC5947", channel: int) -> None:
self._tlc5947 = tlc5947
self._channel = channel

@property
def duty_cycle(self):
def duty_cycle(self) -> int:
"""Get and set the 16-bit PWM duty cycle value for this channel."""
raw_value = self._tlc5947._get_gs_value(self._channel)
# Convert to 16-bit value from 12-bits and return it.
return (raw_value << 4) & 0xFFFF

@duty_cycle.setter
def duty_cycle(self, val):
def duty_cycle(self, val: int) -> None:
if val < 0 or val > 65535:
raise ValueError(
"PWM intensity {0} outside supported range [0;65535]".format(val)
Expand All @@ -96,7 +96,7 @@ def duty_cycle(self, val):
self._tlc5947._set_gs_value(self._channel, val)

@property
def frequency(self):
def frequency(self) -> int:
"""Frequency of the PWM channel, note you cannot change this and
cannot read its exact value (it varies from 2.4-5.6 mhz, see the
TLC5947 datasheet).
Expand All @@ -108,12 +108,19 @@ def frequency(self):
# Must disable a few checks to make pylint happy (ugh).
# pylint: disable=no-self-use,unused-argument
@frequency.setter
def frequency(self, val):
def frequency(self, val: int) -> None:
raise RuntimeError("Cannot set TLC5947 PWM frequency!")

# pylint: enable=no-self-use,unused-argument

def __init__(self, spi, latch, *, auto_write=True, num_drivers=1):
def __init__(
self,
spi: "SPI",
latch: "DigitalInOut",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm don't think I know the difference (if there is one). But I think for most of the libraries we have been using the object imports where possible instead of strings. So these two types could be:

imports:

try:
    import typing
    from busio import SPI
    from digitalio import DitialInOut
except ImportError:
    pass

then for the types:

spi: SPI
latch: DigitalInOut

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the feedback. The only problem I face is the TLC5947 object in the PWMOut constructor, because we have a forward reference. In order to get rid of enclausing the TLC5947 type in quotes we must use from __future__ import annotations. But this line must occur at the beginning of the file, so we can't use a try except block. Should I keep the TLC5947 type annotation as is or should I add the above line outside of a try except block?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep, keep the TLC5947 annotation as it is now.

That one we have to do it with the string because we can't have that import outside of the try block because it will cause an exception if it runs on the micro controllers.

*,
auto_write: bool = True,
num_drivers: int = 1
) -> None:
if num_drivers < 1:
raise ValueError(
"Need at least one driver; {0} is not supported.".format(num_drivers)
Expand All @@ -130,7 +137,7 @@ def __init__(self, spi, latch, *, auto_write=True, num_drivers=1):
# any channel value change).
self.auto_write = auto_write

def write(self):
def write(self) -> None:
"""Write out the current channel PWM values to the chip. This is only
necessary to call if you disabled auto_write in the initializer,
otherwise write is automatically called on any channel update.
Expand All @@ -152,7 +159,7 @@ def write(self):
# Ensure the SPI bus is unlocked.
self._spi.unlock()

def _get_gs_value(self, channel):
def _get_gs_value(self, channel: int) -> int:
# pylint: disable=no-else-return
# Disable should be removed when refactor can be tested
if channel < 0 or channel >= _CHANNELS * self._n:
Expand Down Expand Up @@ -182,7 +189,7 @@ def _get_gs_value(self, channel):
else:
raise RuntimeError("Unsupported bit offset!")

def _set_gs_value(self, channel, val):
def _set_gs_value(self, channel: int, val: int) -> None:
if channel < 0 or channel >= _CHANNELS * self._n:
raise ValueError(
"Channel {0} not available with {1} board(s).".format(channel, self._n)
Expand Down Expand Up @@ -223,7 +230,7 @@ def _set_gs_value(self, channel, val):
if self.auto_write:
self.write()

def create_pwm_out(self, channel):
def create_pwm_out(self, channel: int) -> "PMWOut":
"""Create an instance of a PWMOut-like class that mimics the built-in
CircuitPython PWMOut class but is associated with the TLC5947 channel
that is specified. This PWMOut class has a duty_cycle property which
Expand All @@ -237,19 +244,19 @@ def create_pwm_out(self, channel):
# Define index and length properties to set and get each channel's raw
# 12-bit value (useful for changing channels without quantization error
# like when using the PWMOut mock class).
def __len__(self):
def __len__(self) -> int:
"""Retrieve the total number of PWM channels available."""
return _CHANNELS * self._n # number channels times number chips.

def __getitem__(self, key):
def __getitem__(self, key: int) -> int:
"""Retrieve the 12-bit PWM value for the specified channel (0-max).
max depends on the number of boards.
"""
if key < 0: # allow reverse adressing with negative index
key = key + _CHANNELS * self._n
return self._get_gs_value(key) # does parameter checking

def __setitem__(self, key, val):
def __setitem__(self, key: int, val: int) -> None:
"""Set the 12-bit PWM value (0-4095) for the specified channel (0-max).
max depends on the number of boards.
If auto_write is enabled (the default) then the chip PWM state will
Expand Down