search

adafruit / Adafruit_CircuitPython_MCP2515

A CircuitPython library for working with the MCP2515 CAN bus controller
MIT License
20 stars 14 forks source link

Missing Type Annotations #11

Open FoamyGuy opened 2 years ago

FoamyGuy commented 2 years ago

There are missing type annotations for some functions in this library.

The typing module does not exist on CircuitPython devices so the import needs to be wrapped in try/except to catch the error for missing import. There is an example of how that is done here:

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

Once imported the typing annotations for the argument type(s), and return type(s) can be added to the function signature. Here is an example of a function that has had this done already:

def wrap_text_to_pixels(
    string: str, max_width: int, font=None, indent0: str = "", indent1: str = ""
) -> List[str]:

If you are new to Git or Github we have a guide about contributing to our projects here: https://learn.adafruit.com/contribute-to-circuitpython-with-git-and-github

There is also a guide that covers our CI utilities and how to run them locally to ensure they will pass in Github Actions here: https://learn.adafruit.com/creating-and-sharing-a-circuitpython-library/check-your-code In particular the pages: Sharing docs on ReadTheDocs and Check your code with pre-commit contain the tools to install and commands to run locally to run the checks.

If you are attempting to resolve this issue and need help, you can post a comment on this issue and tag both @foamyguy and @kattni or reach out to us on Discord: https://adafru.it/discord in the #circuitpython-dev channel.

The following locations are reported by mypy to be missing type annotations:

tcfranks commented 1 year ago

@FoamyGuy I ran into something I'm trying to wrap my head around. in adafruit_mcp2515.init.py line 549 def _set_filter_register(self, filter_index, mask, extended): filter_reg_addr = FILTERS[filter_index] self._write_id_to_register(filter_reg_addr, mask, extended)

trying to type filter_index, since FILTERS[0] or FILTERS[1] both return a list of ints, but _write_id_to_register turns right around and assigns filter_reg_addr to the second byte of a bytearray, so clearly we think we hitting FILTERS like FILTERS[1][2] and returning a single int? what should filter_index really look like? I can't find any internal references to this function, nor in any of the source or example files - could it just be a bug? or is the problem inside my head?

tcfranks commented 1 year ago

also, searching adafruit for mcp2515 turns up nothing, so if I were to get a product to sort this out, is it something included in another product perhaps?

FoamyGuy commented 1 year ago

@tcfranks good question on the product. I had a look around and came up empty so far. My best guess is maybe this was for a breakout or device that didn't end up being made/sold.

There are a few M4 devices with CAN builtin like: https://www.adafruit.com/product/4759 which is the best guess I've got. But as far as I can tell they don't actually use this library in the example code provided with them.

As for the typing of _set_filter_register. I don't see any usages of that either, I think you are correct about there not being any. For the type of filter_index it looks like int to me since it's always being used to access an item that is in a list. It would be the 1 or 0 in something like FILTERS[0] or FILTERS[1]. If I am understanding correctly, the type of whatever value comes out of the FILTERS list from the index we accessed doesn't influence the type of filter_index.

I will ask around to see if I can find out whether there is any available hardware that uses this library.

FoamyGuy commented 1 year ago

The library could maybe be used with: https://www.sparkfun.com/products/13262

Perhaps there was a similar featherwing or other breakout in the works at some point.

tcfranks commented 1 year ago

I'm punting this. Looks like there was a 3rd party featherwing at one time other than the sparkfun shield.