#We re working on getting mypy working

1 messages · Page 1 of 1 (latest)

wide gale
#

I'm working on Adafruit_CircuitPython_BLE_MIDI (maybe that's too complex, but we'll see!) and I just installed the libs from the requirements.txt. That specifically installed:

Adafruit-Blinka-7.2.2 Adafruit-PlatformDetect-3.22.1 Adafruit-PureIO-1.1.9 adafruit-blinka-bleio-2.2.0 adafruit-circuitpython-ble-8.2.4 bleak-0.11.0 dbus-next-0.2.3 pyftdi-0.54.0 pyserial-3.5 pyusb-1.2.1

placid tinsel
#

@left ginkgo

left ginkgo
placid tinsel
#

Yes, that is correct

wide gale
#

@placid tinsel I could do a pip install -r requirements.txt without error in your repo also. Reported installed packages:

Adafruit-Blinka-7.2.2 Adafruit-PlatformDetect-3.22.1 Adafruit-PureIO-1.1.9 adafruit-blinka-bleio-2.2.0 adafruit-circuitpython-ble-8.2.4 bleak-0.11.0 dbus-next-0.2.3 pyftdi-0.54.0 pyserial-3.5 pyusb-1.2.1

left ginkgo
#

It was a bit tricky to track it down but I followed it back to where matches() gets called from and noticed that makes reference to ScanEntry class. Then I checked the docs to verify that class does have advertisement_bytes property which is what's beeing accessed from inside of matches() so that seems correct to me. I happen to be streaming today so there is a video showing the path I took to find it out if your interested in my thought processes.

placid tinsel
#

So, when I tried to find references to matches, I wasn't able to find anything that calls it. How were you able to find it?

left ginkgo
#

@wide gale I'm not sure I understand fully. You are trying to get the requirements set up to run the mypy check locally on your machine? And having trouble getting the right things installed? or something else?

placid tinsel
#

def matches(cls, entry: _bleio.ScanEntry) -> bool:

left ginkgo
placid tinsel
#

Okay, where could I find your stream? On discord?

left ginkgo
placid tinsel
#

Thanks so much, I'm watching now

left ginkgo
#

I think around -17:00 ish in the YT stream is when I first started looking into yours, it doesn't seem to want to let me copy a link with tampstamp in it though.

placid tinsel
#

Okay, I've found it, thanks so much. Watching now!

placid tinsel
#

@left ginkgo, I want to try to walk through a fix real quick. On that same file I was on before would this be a correct fix for line 88:

#

@msg.setter def msg(self, value: const): self.manufacturer_data.data[_RADIO_DATA_ID] = value

#

I arrived at the const because it's set higher up in the file

#

Yes, bleradio

#

value is set higher as like value = const(00x00)

#

No, I lied, it was something else that was set as that

#

Yes, one of those

left ginkgo
#

I think it should be like this.

from circuitpython_typing import ReadableBuffer
#...
def msg(self, value: ReadableBuffer)
placid tinsel
#

Okay, I imagine then that I can go and sleuth through the rest of those functions and suss out whether they're readablebuffer or writeablebuffer depending on what will happen to them. Does that stand to reason to you? I picked a tough first contribution haha

#

Okay, cool. I'll also make sure to have someone here look it over as well

left ginkgo
#

@boreal spindle if you are around and have a sec. Does it make sense to you that value argument here would be ReadableBuffer type? it appears to be bytes from what I can tell but I think it would support any of the ones from that protocol if I understand correctly.

placid tinsel
#

@left ginkgo Thank you for bearing with me as I work on this stuff. I'm now trying to chase down msg(self) on 81. I'm thinking that it should look like def msg(self: const) -> tuple[str, const]: after looking up higher in the file and seeing that _RADIO_DATA_ID is set to const(0x0001)

#

I can leave it as it is, but could you see if my thinking was correct?

#

So more properly it would just return bytes

left ginkgo
boreal spindle
#

@left ginkgo I think it should it be typing.Tuple, since using builtins was only added in 3.9

left ginkgo
boreal spindle
#

For @placid tinsel's return type, sorry

#

Oh,

#

I am bad at reading

#

Nevermind if it returns something else 😅

left ginkgo
#

Ah, the first line here is a bytes so I assume that should be at least one of the valid types. I'm not sure about the second return line but doesn't seem to get put into a tuple that I can tell.

left ginkgo
placid tinsel
#

Are you stll looking at this window?

#

@left ginkgo Could I show you the fix request and send you my updated file and see if I'm on the right track with properly implementing a fix?

left ginkgo
placid tinsel
#

Haha, awesome thanks

left ginkgo
placid tinsel
#

Okay, that's removed

#

So is it not actually doing they "typing" when it runs?

#

Yeah, sorry, on the microcontroller

#

I thought the self ones didnt need typed?

boreal spindle
placid tinsel
#

Ahhh okay, cool

#

I just skipped the **args

#

Yeah. that was absolutely my thinking as well

boreal spindle
#

It may still be okay to use builtins, just not as generics. I think you had a type annotation recently that use tuple, which I think is fine because they weren't parameterized (tuple[int, int])

#

^ @left ginkgo, that is

placid tinsel
#

the float docstring was there already

#

Thank you so much, I really appreciate it! What a tricky first contribution haha

#

No thanks, I've been updated as we've been going

boreal spindle
#

Hey @placid tinsel I'm taking a look at your BLE_Radio PR now!

placid tinsel
#

Awesome, we're actually pushing it again so that it passes all tests, maybe wait 5 minutes

boreal spindle
#

Sounds good!

placid tinsel
#

Ready to review!

left ginkgo
#

I see there have been some other PRs submitted so far today as well. I'll plan on taking a look over any that are open still this afternoon after the meeting and I'll turn the stream back on while I go over them if anyone is interested in watching for live feedback.

placid tinsel
#

@left ginkgo No rush or anything, but I've got another fix I think may be ready to commit. Could we take a look together?

left ginkgo
placid tinsel
#

Awesome, thank you so much!

boreal spindle
left ginkgo
#

@placid tinsel just started the stream back up and will take a look at your newer one.

placid tinsel
#

Awesome, thank you!

#

yes

#

oh no, actually the committed stuff is further along

#

github is the best version

#

service: Optional[Service]?

#

We were actually fuzzy on the error thing over here as well

#

okay cool

#

okay, so is it the same for the last func then? qhere otherwise there is no return?

boreal spindle
placid tinsel
#

It should be MagicLightService?

#

Optional[MLS]

#

Also, is that optional you're looking at right now the string?

left ginkgo
#

Optional["MagicLightService"]

boreal spindle
#

party_blob I have you pulled up, always nice to follow along!

placid tinsel
#

Yeahhhh would it be like that or like Optional[MLS]

#

without quotes

boreal spindle
#

It would be with quotes because when a type checker is parsing a file, it reads through it to do so. Since the actually class we need to type with would still be in the process of being imported we need to let the type checker know what the type return will be without actually using it. That's where the quotes come in.

placid tinsel
#

Thank you for this explanation

#

It sounds like we don't type exceptions

#

Currently the only reason to do so would be docs

#

So just put it in the docs, that's basically what they say

#

I imagine it would make sense to type it someday, yeah

#

Totally possible I did some weird stuff on my end

#

I totally could have messed up git stuff on my end

#

I was definitely like cloning and forking and checkout and such all over the place

#

Okay, cool. Haha, I just don't wanna be accidentally breaking anything. But it looks good

#

Thank you so much for all this help. I'm finally a contributor! Feels like a pretty cool step for me

#

Of course! I'm stoked to do it

boreal spindle
#

I think it's how it works for all of them, that the service is the specific service I thiiiiink that because of how the ble library works for both sides of the BLE??? I think I had talked to Dan about it...

#

I wish I knew more about BLE, it's just invisible tubes to me upsidedown_cry

#

Yup!

placid tinsel
#

@left ginkgo Could we look at one last example together before I take the training wheels off?

#

Absolutely, no rush at all

#

json web token

#

Ahhh okay

boreal spindle
#

hallo

placid tinsel
#

what exactly would be the difference?

#

ahhhh okay. I bet that's gonna replace something I used later haha

boreal spindle
#

the b2a_base64 can take any ReadableBuffer and will spit out a bytes object, and that looks like the only limiting factor, so I agree

#

ReadableBuffer should be the type for it 🙂

placid tinsel
#

for both input and output?

boreal spindle
#

Yeah, CircuitPython happens to implement str similar to the buffer protocol, but that isn't the same behavior as CPython

placid tinsel
#

yeah

#

it does

#

i ran that code

#

type(str.encode('ascii'))
<class 'bytes'>

#

Yeah, it was super helpful. Thank you

#

where does readablebuffer import from?

boreal spindle
#

You'll need to make sure it's included in setup.py and requirements.txt

#

I helped someone with that earlier, I'll link that

placid tinsel
#

` def init(
self,
requests,
client_id: str,
client_secret: str,
scopes: list,
access_token: Optional[str]=None,
refresh_token: Optional[str]=None,
) -> None:
self._requests = requests
self._client_id = client_id
self._client_secret = client_secret
self._scopes = scopes

    # A value that Google uniquely assigns to identify the device
    self._device_code = None
    # The length of time that the codes above are valid, in seconds
    self._expiration_time = None
    # The length of time we'll wait between polling the auth. server, in seconds
    self._interval = None
    # A url user must navigate to on a browser
    self.verification_url = None
    # Identifies the scopes requested by the application
    self.user_code = None
    # The remaining lifetime of the access token, in seconds
    self.access_token_expiration = None
    # The scopes of access granted by the access_token as a list
    self.access_token_scope = None

    # The token that your application sends to authorize a Google API request
    self.access_token = access_token

    # A token that you can use to obtain a new access token
    # Refresh tokens are valid until the user revokes access
    self.refresh_token = refresh_token`
📎 message.txt
#

And so my question is: Should the new code simply import that from wherever it comes from and the requests line look like:

` requests: adafruit_requests.request,

left ginkgo
# placid tinsel And so my question is: Should the new code simply import that from wherever it c...

I think the annotation will be:

requests: adafruit_requests.Session

Inside of __init__ function.

The simpletest example here: https://github.com/adafruit/Adafruit_CircuitPython_OAuth2/blob/a1073fb5ba8c1b93a851c99fd43ca2fef00e6fc4/examples/oauth2_simpletest.py#L25-L34

Creates and passes in this object.

GitHub
Adafruit_CircuitPython_OAuth2/oauth2_simpletest.py at a1073fb5ba8c1...

OAuth 2.0 authorization helper for accessing Google APIs - Adafruit_CircuitPython_OAuth2/oauth2_simpletest.py at a1073fb5ba8c1b93a851c99fd43ca2fef00e6fc4 · adafruit/Adafruit_CircuitPython_OAuth2

boreal spindle
#

Sorry, just saw this! But definitely second what @left ginkgo responded with.

wide gale
#

@left ginkgo regarding https://github.com/adafruit/Adafruit_Blinka_bleio/pull/44 - I've been reflecting that this is probably NOT the highest bang-per-buck thing to work on and I should maybe pop up one level to the actual circuitpython BLE codebase?

That said, I'm going to focus on cleaning up what's there for now, and I responded to your comments. I welcome your course corrections and nudges!

GitHub
Improved type hints by davclark · Pull Request #44 · adafruit/Adafr...

Changes

Numerous typing changes to reduce number of mypy complaints

Please check my work! My focus was on quieting mypy, but downstream usage may differ (e.g., I changed int to Attribute in Chara...

left ginkgo
# wide gale <@382939733107408897> regarding https://github.com/adafruit/Adafruit_Blinka_blei...

I would say work on whichever you are more comfortable or interested in. If you mean BLE inside of the circuitpython core you'll probably need more familiarity with C than python if that makes any difference to you.

And sounds good. I'm about to get some lunch but I'll have another look over this afternoon and mention anything else that comes to mind.

Thanks for working on these!

wide gale
frosty shadow
#

_bleio is implemented natively for a couple of ports, and is also implemented in Python for use on host computers.

#

so there are both Python and C implementations of _bleio. It is a low-level interface, which is mostly not meant to be used by the user, and we don't guarantee API compatibility, hence the leading underscore.

wide gale
#

So specifically for the blinka _bleio, my understanding was that I was targeting CPython, and not CircuitPython... that seems consistent with what you're saying? Am I missing hardware ports implemented within the https://github.com/adafruit/Adafruit_Blinka_bleio repo?

Probably the most useful point of understanding would be if _bleio is implemented for a bluefruit... I have one of those that I can continue working with at home.

left ginkgo
#

If you mean CircuitPlayground Bluefruit. I do think _bleio is implemented for it. But I'm not super experience with BLE I may understand incorrectly.

frosty shadow
#

I mentioned the core implementations so you could crib the signatures from there.

#

yes, the hw ports are all inside adafruit/circuitpython

frosty shadow
#

@wide gale ^^

wide gale
#

OK - so I took a step back for lunch and I think I figured out my confusion. This would (I think) be true in general for any blinka stuff, but I'll just talk about _bleio.

I may be overthinking this, so feel free to punt! But it seems like wrapping my head around this will help me contribute effectively.

wide gale
# wide gale OK - so I took a step back for lunch and I think I figured out my confusion. Thi...

Right now, there are multiple implementations of _bleio. Zooming in, there's the Characteristic.add_to_service() python function defined in Adafruit_Blinka_bleio/_bleio/common.py and also in circuitpython/shared-bindings/_bleio/Characteristic.c (as the C function bleio_characteristic_add_to_service()).

In the blinka python code, Attribute is defined as an enum.Enum, and in the higher-level BLE python code the values of Attribute are specified as class attributes (not an Enum) based on what's found in _bleio.Attribute. So, on the cpython / blinka side, values here are also boxed Enums, not ints.

wide gale
# wide gale Right now, there are multiple implementations of _bleio. Zooming in, there's the...

In the C code, there's a C enum bleio_attribute_security_mode_t defined in shared-module/_bleio/Attribute.h - I guess this is the "real" underlying type for circuitpython, and in C enums are implemented as int. So, that Attribute class in the BLE code would end up pointing to the C enum values (which are ints).

This is where the disconnect comes from for me. I think the implementation is right (obviously - it works!) - the question is simply what values to encourage users to use (or not) when specifying arguments to Characteristic.add_to_service().

I need to do some more digging if this is reasonable, but I'd push for a vision where circuitpython supports a similar typing syntax to CPython, where we could use an actual Enum type signature and that would work also on Circuitpython. I wonder if we could just import and use _bleio.Attribute instead of creating a proxy class. In circuitpython, _bleio.Attribute would be an alias for bleio_attribute_security_mode_t (or int), and with blinka it'd be a python Enum.

From a "user" perspective, the idea is that calling code should be written as Attribute.SECURITY_MODE_NO_ACCESS, not 0x00. So I'm imagining a world where mypy compains when you do it the second way, even though it'd work in circuitpython.

wide gale
frosty shadow
#

@wide gale Thanks for thinking this through. I didn't remember there were real Python enums in the Python _bleio. When I implemented _bleio in Python, we were not doing type signatures, so it didn't really matter. We do have mechanism for defining objects in an enum-ish way in CircuitPython; there are macros for that: https://github.com/adafruit/circuitpython/blob/main/py/enum.h and .c. I don't remember why I didn't use that in _bleio, but it would be possible. Alternatively we could just use ints in the Python library.

wide gale
#

Shall we wait until the dev meeting next Monday to socialize a general path forward?