Type annotations

[HundredVisionsGuy]

Still failing mypy and I have some questions to pose regarding the quaternion_packet.py script on line 44.

I need to head home, but I will continue working on this in the coming days. Here are the mypy results:

adafruit_bluefruit_connect\packet.py:70: error: On Python 3 formatting "b'abc'" with "{}" produces "b'abc'", not "abc"; use "{!r}" if this is desired behavior  [str-bytes-safe]
adafruit_bluefruit_connect\packet.py:130: error: On Python 3 formatting "b'abc'" with "{}" produces "b'abc'", not "abc"; use "{!r}" if this is desired behavior  [str-bytes-safe]
adafruit_bluefruit_connect\color_packet.py:45: error: Argument 1 to "len" has incompatible type "Optional[Packet]"; 
expected "Sized"  [arg-type]
adafruit_bluefruit_connect\color_packet.py:45: error: Item "Packet" of "Optional[Packet]" has no attribute "__iter__" (not iterable)  [union-attr]
adafruit_bluefruit_connect\color_packet.py:45: error: Item "None" of "Optional[Packet]" has no attribute "__iter__" 
(not iterable)  [union-attr]
adafruit_bluefruit_connect\color_packet.py:46: error: Cannot determine type of "_color"  [has-type]
adafruit_bluefruit_connect\color_packet.py:51: error: Argument 1 of "parse_private" is incompatible with supertype "Packet"; supertype defines the argument type as "bytes"  [override]
adafruit_bluefruit_connect\color_packet.py:51: note: This violates the Liskov substitution principle
adafruit_bluefruit_connect\color_packet.py:51: note: See https://mypy.readthedocs.io/en/stable/common_issues.html#incompatible-overrides
adafruit_bluefruit_connect\color_packet.py:56: error: Argument 1 to "ColorPacket" has incompatible type "Tuple[Any, 
...]"; expected "Optional[Packet]"  [arg-type]
adafruit_bluefruit_connect\color_packet.py:56: error: Argument 2 to "unpack" has incompatible type "Optional[Packet]"; expected "Union[bytes, Union[bytearray, memoryview, array[Any], mmap, _CData, PickleBuffer]]"  [arg-type]        
adafruit_bluefruit_connect\color_packet.py:61: error: Cannot determine type of "_color"  [has-type]
adafruit_bluefruit_connect\color_packet.py:69: error: Cannot determine type of "_color"  [has-type]
adafruit_bluefruit_connect\quaternion_packet.py:44: error: Unsupported operand types for + ("bytes" and "int")  [operator]
Found 12 errors in 3 files (checked 11 source files)```

[FoamyGuy]

resolves: #31

[FoamyGuy]

Thanks for working on this!

Nice find on that issue at line 44 in quaternion packet. I'm not super familiar with this library, but after having a look around I am thinking that might be a bug. The code it warns about is this:

        partial_packet = struct.pack(
            self._FMT_CONSTRUCT, self._TYPE_HEADER, self._x, self._y, self._z, self._w
        )
        return partial_packet + self.checksum(partial_packet)

Which does look to me like it would be trying add a bytes() and an int which as the error states isn't really allowed.

I'm thinking it should be doing something like inside of here in the base class: https://github.com/adafruit/Adafruit_CircuitPython_BluefruitConnect/blob/863951fb2c4b375f627982d5dafefd46022a2440/adafruit_bluefruit_connect/packet.py#L143-L147

And I'm guessing it hasn't been used in the right way for anyone to run into that issue (or at least not reported it if so).

Regarding the other mypy warnings: It's okay at this point if we're not passing 100% of the mypy checks. This first pass is aimed at getting the "low hanging fruit" of function arguments and returns. Once we get all of that taken care of eventually we'll enable automatic mypy check and decide on a configuration for it at that point with which warnings will be active in the automated check

[HundredVisionsGuy]

Shall I go ahead and make the PR (it's currently in draft status) even with the warning on quaternion_packet.py for line 44?

FYI: When I run mypy on the entire folder, I just have 3 errors, and only the last one is related to the issue regarding quaternion_packet.py

I am wondering if we want to add the to_bytes() method on the self.checksum(partial_packet)

adafruit_bluefruit_connect\packet.py:70: error: On Python 3 formatting
"b'abc'" with "{}" produces "b'abc'", not "abc"; use "{!r}" if this is
desired behavior  [str-bytes-safe]
adafruit_bluefruit_connect\packet.py:130: error: On Python 3 formatting
"b'abc'" with "{}" produces "b'abc'", not "abc"; use "{!r}" if this
is desired behavior  [str-bytes-safe]
adafruit_bluefruit_connect\quaternion_packet.py:44: error: Missing
positional arguments "length", "byteorder" in call to "to_bytes" of "int"
 [call-arg]
Found 3 errors in 2 files (checked 11 source files)

On Wed, Apr 26, 2023, 4:35 PM foamyguy @.***> wrote:

Thanks for working on this!

Nice find on that issue at line 44 in quaternion packet. I'm not super familiar with this library, but after having a look around I am thinking that might be a bug. The code it warns about is this:

    partial_packet = struct.pack(
        self._FMT_CONSTRUCT, self._TYPE_HEADER, self._x, self._y, self._z, self._w
    )
    return partial_packet + self.checksum(partial_packet)

Which does look to me like it would be trying add a bytes() and an int which as the error states isn't really allowed.

I'm thinking it should be doing something like inside of here in the base class:

https://github.com/adafruit/Adafruit_CircuitPython_BluefruitConnect/blob/863951fb2c4b375f627982d5dafefd46022a2440/adafruit_bluefruit_connect/packet.py#L143-L147

And I'm guessing it hasn't been used in the right way for anyone to run into that issue (or at least not reported it if so).

Regarding the other mypy warnings: It's okay at this point if we're not passing 100% of the mypy checks. This first pass is aimed at getting the "low hanging fruit" of function arguments and returns. Once we get all of that taken care of eventually we'll enable automatic mypy check and decide on a configuration for it at that point with which warnings will be active in the automated check

— Reply to this email directly, view it on GitHub https://github.com/adafruit/Adafruit_CircuitPython_BluefruitConnect/pull/37#issuecomment-1524198933, or unsubscribe https://github.com/notifications/unsubscribe-auth/AA5GKUUIMANNMTMBBFEVQ3LXDGWN3ANCNFSM6AAAAAAXM2NZGI . You are receiving this because you authored the thread.Message ID: <adafruit/Adafruit_CircuitPython_BluefruitConnect/pull/37/c1524198933@ github.com>

[FoamyGuy]

I do think it's okay to take the PR out of draft mode even though the line 44 thing is still outstanding.

You can go ahead and make the change on line 44 if you want I think we'd be wrapping the self.checksum() inside of a call to bytes() just like the one in the code linked above.

We'll try to get someone with more familiarity with this library to take a look and verify that it makes sense.

[HundredVisionsGuy]

Closes #37

NOTE: on quaternion_packet.py to_bytes() line 44, I wrapped self.checksum(partial_packet, ) in bytes() per base class's to_bytes() (packet.py) per suggestion by @FoamyGuy .