Skip to content
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.

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

Rfcomm: Change dbus type signature to an int #2492

Merged
merged 1 commit into from
Sep 18, 2024

Conversation

infirit
Copy link
Contributor

@infirit infirit commented Sep 13, 2024

Fixes: #2489

Copy link

sonarcloud bot commented Sep 13, 2024

Copy link
Member

@cschramm cschramm left a comment

Choose a reason for hiding this comment

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

I'm still wondering why this did not come up yet. Are there any specific conditions that cause the error? 🤔

@infirit infirit merged commit e1c1bf6 into blueman-project:main Sep 18, 2024
21 checks passed
@infirit infirit deleted the dbusrfcommint branch September 18, 2024 20:20
@infirit
Copy link
Contributor Author

infirit commented Sep 18, 2024

Not sure. I usually don't use serial devices/rfcomm so I probably never noticed. I completely relied on the the type annotation while I should have done at least a quick test.

@cschramm
Copy link
Member

Oh, I get it. This only became an issue with 5ea6542, thus the 2.4 release. I expected an issue that we'd have had for much longer, but it's only due to the f-string.

@infirit
Copy link
Contributor Author

infirit commented Sep 19, 2024

Yeah, I assumed the type hint was correct. Maybe we can change the add_method function to require python types. We might then be able to change the annotation and be more specific than Callable[..., Any]. So mypy can help us (mostly me lol) spot these issues.

Maybe a TypeVar if we pass a proper type or maybe even simpler NewType of str for each dbus typestring 🤔

I may experiment with that when I'm done with the for main PR.

Edit: this looks like a solution but its 3.10 or later https://docs.python.org/3.10/library/typing.html#typing.ParamSpec

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants