Skip to content

Add Missing Type Annotations #40

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.

Sign up for GitHub

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
Oct 24, 2022
Merged

Add Missing Type Annotations #40

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
48a26e1
Add Missing Type Annotations
tcfranks Oct 10, 2022
05f595f
fix job run error - Replace a List with Tuple
tcfranks Oct 10, 2022
49ca754
fix job run error - Replace a List with Tuple
tcfranks Oct 17, 2022
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
56 changes: 33 additions & 23 deletions adafruit_sgp30.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,12 @@
from adafruit_bus_device.i2c_device import I2CDevice
from micropython import const

try:
from typing import List, Tuple
from busio import I2C
except ImportError:
pass

__version__ = "0.0.0+auto.0"
__repo__ = "https://github.com/adafruit/Adafruit_CircuitPython_SGP30.git"

Expand Down Expand Up @@ -80,7 +86,7 @@ class Adafruit_SGP30:

"""

def __init__(self, i2c, address=_SGP30_DEFAULT_I2C_ADDR):
def __init__(self, i2c: I2C, address: int = _SGP30_DEFAULT_I2C_ADDR) -> None:
"""Initialize the sensor, get the serial # and verify that we found a proper SGP30"""
self._device = I2CDevice(i2c, address)

Expand All @@ -94,61 +100,63 @@ def __init__(self, i2c, address=_SGP30_DEFAULT_I2C_ADDR):

@property
# pylint: disable=invalid-name
def TVOC(self):
def TVOC(self) -> int:
"""Total Volatile Organic Compound in parts per billion."""
return self.iaq_measure()[1]

@property
# pylint: disable=invalid-name
def baseline_TVOC(self):
def baseline_TVOC(self) -> int:
"""Total Volatile Organic Compound baseline value"""
return self.get_iaq_baseline()[1]

@property
# pylint: disable=invalid-name
def eCO2(self):
def eCO2(self) -> int:
"""Carbon Dioxide Equivalent in parts per million"""
return self.iaq_measure()[0]

@property
# pylint: disable=invalid-name
def baseline_eCO2(self):
def baseline_eCO2(self) -> int:
"""Carbon Dioxide Equivalent baseline value"""
return self.get_iaq_baseline()[0]

@property
# pylint: disable=invalid-name
def Ethanol(self):
def Ethanol(self) -> int:
"""Ethanol Raw Signal in ticks"""
return self.raw_measure()[1]

@property
# pylint: disable=invalid-name
def H2(self):
def H2(self) -> int:
"""H2 Raw Signal in ticks"""
return self.raw_measure()[0]

def iaq_init(self):
def iaq_init(self) -> List[int]:
"""Initialize the IAQ algorithm"""
# name, command, signals, delay
self._run_profile(["iaq_init", [0x20, 0x03], 0, 0.01])
self._run_profile(("iaq_init", [0x20, 0x03], 0, 0.01))

def iaq_measure(self):
def iaq_measure(self) -> List[int]:
"""Measure the eCO2 and TVOC"""
# name, command, signals, delay
return self._run_profile(["iaq_measure", [0x20, 0x08], 2, 0.05])
return self._run_profile(("iaq_measure", [0x20, 0x08], 2, 0.05))

def raw_measure(self):
def raw_measure(self) -> List[int]:
"""Measure H2 and Ethanol (Raw Signals)"""
# name, command, signals, delay
return self._run_profile(["raw_measure", [0x20, 0x50], 2, 0.025])
return self._run_profile(("raw_measure", [0x20, 0x50], 2, 0.025))

def get_iaq_baseline(self):
def get_iaq_baseline(self) -> List[int]:
"""Retreive the IAQ algorithm baseline for eCO2 and TVOC"""
# name, command, signals, delay
return self._run_profile(["iaq_get_baseline", [0x20, 0x15], 2, 0.01])
return self._run_profile(("iaq_get_baseline", [0x20, 0x15], 2, 0.01))

def set_iaq_baseline(self, eCO2, TVOC): # pylint: disable=invalid-name
def set_iaq_baseline( # pylint: disable=invalid-name
self, eCO2: int, TVOC: int
) -> None:
"""Set the previously recorded IAQ algorithm baseline for eCO2 and TVOC"""
if eCO2 == 0 and TVOC == 0:
raise RuntimeError("Invalid baseline")
Expand All @@ -157,19 +165,19 @@ def set_iaq_baseline(self, eCO2, TVOC): # pylint: disable=invalid-name
arr = [value >> 8, value & 0xFF]
arr.append(self._generate_crc(arr))
buffer += arr
self._run_profile(["iaq_set_baseline", [0x20, 0x1E] + buffer, 0, 0.01])
self._run_profile(("iaq_set_baseline", [0x20, 0x1E] + buffer, 0, 0.01))

def set_iaq_humidity(self, gramsPM3): # pylint: disable=invalid-name
def set_iaq_humidity(self, gramsPM3: float) -> None: # pylint: disable=invalid-name
"""Set the humidity in g/m3 for eCO2 and TVOC compensation algorithm"""
tmp = int(gramsPM3 * 256)
buffer = []
for value in [tmp]:
arr = [value >> 8, value & 0xFF]
arr.append(self._generate_crc(arr))
buffer += arr
self._run_profile(["iaq_set_humidity", [0x20, 0x61] + buffer, 0, 0.01])
self._run_profile(("iaq_set_humidity", [0x20, 0x61] + buffer, 0, 0.01))

def set_iaq_relative_humidity(self, celsius, relative_humidity):
def set_iaq_relative_humidity(self, celsius: float, relative_humidity: float):
"""
Set the humidity in g/m3 for eCo2 and TVOC compensation algorithm.
The absolute humidity is calculated from the temperature (Celsius)
Expand All @@ -185,7 +193,7 @@ def set_iaq_relative_humidity(self, celsius, relative_humidity):

# Low level command functions

def _run_profile(self, profile):
def _run_profile(self, profile: Tuple[str, List[int], int, float]) -> List[int]:
Copy link
Member

Choose a reason for hiding this comment

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

I didn't review everything but thought it would be helpful to mention this first - technically this library uses lists for profile instead of tuple so it's not correct. However, the way that this function unpacks the argument, I think tuples are a much better fit, and we should change all uses of this function to use tuples instead of lists. Is that something you would be interesting in help with? It'll make the type annotations much smoother and sensible.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

well, based on a comment to another pr, wouldn't we fix this to be lists for annotations and do the change to tuple in another pr? I think I used tuple here because of hints pycharm was giving me and also the fact I was having problems wrapping my head around the structure of the profile being a list. which says more about me and my understanding of lists, I suppose. so yes, I should dig a bit and do that. not before next week, tho. In here or a different PR?

Copy link
Member

Choose a reason for hiding this comment

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

I would do it here, unlike the other PR, changing those lists to tuples wouldn't be API-breaking, just minor modifications to aid in type annotations. I agree with you that that it just doesn't make sense for it to be a list, since we're unpacking it to a fixed set of variables anyway.

"""Run an SGP 'profile' which is a named command set"""
# pylint: disable=unused-variable
name, command, signals, delay = profile
Expand All @@ -195,7 +203,9 @@ def _run_profile(self, profile):
# (name, ["0x%02x" % i for i in command], signals, delay))
return self._i2c_read_words_from_cmd(command, delay, signals)

def _i2c_read_words_from_cmd(self, command, delay, reply_size):
def _i2c_read_words_from_cmd(
self, command: List[int], delay: float, reply_size: int
) -> List[int]:
"""Run an SGP command query, get a reply and CRC results if necessary"""
with self._device:
self._device.write(bytes(command))
Expand All @@ -216,7 +226,7 @@ def _i2c_read_words_from_cmd(self, command, delay, reply_size):
return result

# pylint: disable=no-self-use
def _generate_crc(self, data):
def _generate_crc(self, data: bytearray) -> int:
"""8-bit CRC algorithm for checking data"""
crc = _SGP30_CRC8_INIT
# calculates 8-Bit checksum with given polynomial
Expand Down