Protobuf type interfaces for type-checking & undoing protobufs hack #528
Conversation
| ./nanopb-0.4.7/generator-bin/protoc -I=protobufs --python_out ./ --mypy_out ./ ./protobufs/meshtastic/*.proto | ||
| ./nanopb-0.4.7/generator-bin/protoc -I=protobufs --python_out ./meshtastic/ --mypy_out ./meshtastic/ ./protobufs/nanopb.proto |
There was a problem hiding this comment.
This is what gets the pyi files to be generated.
| from meshtastic import channel_pb2, config_pb2, portnums_pb2, remote_hardware | ||
| from meshtastic import channel_pb2, config_pb2, portnums_pb2, remote_hardware, BROADCAST_ADDR | ||
| from meshtastic.version import get_active_version | ||
| from meshtastic.__init__ import BROADCAST_ADDR |
There was a problem hiding this comment.
Removing this import from meshtastic.__init__ was something mypy needed to be happy, and is done a few different places. Otherwise it would complain about the same file being imported both as meshtastic and as meshtastic.__init__. Plus importing from __init__ is a little bit funky anyway, pythonically speaking.
| import timeago # type: ignore[import-untyped] | ||
| from google.protobuf.json_format import MessageToJson | ||
| from pubsub import pub | ||
| from pubsub import pub # type: ignore[import-untyped] |
There was a problem hiding this comment.
These sorts of ignore[import-untyped] silence mypy a bit when using libraries that don't have type hints (or at least, aren't marked as such) nor type stubs.
| @@ -239,7 +240,7 @@ def getNode(self, nodeId, requestChannels=True): | |||
|
|
|||
| def sendText( | |||
| self, | |||
| text: AnyStr, | |||
There was a problem hiding this comment.
While this was marked as AnyStr (which is basically "either string or bytes, but not a mix of the two"), we treated it afterwards as always being a string. mypy complained about that and I think it's right to have this be a plain str instead.
| @@ -123,7 +123,7 @@ def requestConfig(self, configType): | |||
| print("Requesting current config from remote node (this can take a while).") | |||
|
|
|||
| msgIndex = configType.index | |||
| if configType.containing_type.full_name == "LocalConfig": | |||
| if configType.containing_type.full_name in ("meshtastic.LocalConfig", "LocalConfig"): | |||
There was a problem hiding this comment.
This is the one actual thing that just plain didn't work. The namespacing changed the full name, so trying to fetch things in LocalConfig actually fetched from ModuleConfig.
| server_address = (hostname, portNumber) | ||
| sock = socket.create_connection(server_address) | ||
| self.socket = sock | ||
| self.socket: Optional[socket.socket] = sock |
There was a problem hiding this comment.
This made mypy happier since this could be set to None as well as being an actual socket.
| mypy | ||
| mypy-protobuf | ||
| types-protobuf | ||
| types-tabulate | ||
| types-requests | ||
| types-setuptools | ||
| types-PyYAML |
There was a problem hiding this comment.
Probably self-explanatory, but this adds mypy, the mypy-protobuf plugin that generates the .pyi files, and type stubs for what existed.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #528 +/- ##
==========================================
- Coverage 63.87% 63.83% -0.04%
==========================================
Files 14 14
Lines 2832 2829 -3
==========================================
- Hits 1809 1806 -3
Misses 1023 1023
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
ianmcorvidae
force-pushed
the
protobufs-mypy
branch
5 times, most recently
from
c5e6a84 to
fce31e8
Compare
ianmcorvidae
force-pushed
the
protobufs-mypy
branch
from
fce31e8 to
7a1b4b0
Compare
|
Alright, I'm gonna take the risk and merge this in and maybe/hopefully start adding some more types. Here's hoping it doesn't break everything! |
There's a lot of changes here but the majority of them are generated code. The big thing is that we're not rewriting the protobufs to undo the change to namespaced stuff, and we're adding in a plugin that generates
.pyifiles that denote types for the protobufs. There's also a few other miscellaneous changes that mostly boil down to me making mypy happy and fixing the one bona fide bug I found. I'll leave a review pointing out the non-generated changes.