Skip to content

Conversation

@brenordv
Copy link

@brenordv brenordv commented Sep 30, 2025

Problem

When installing lora-sx126x or lora-sx127x packages via mip, users only get the hardware drivers but not the SyncModem/AsyncModem wrapper classes, making the packages unusable.

Also found a problem in the import handling in lora/__init__.py where ImportError would be raised incorrectly when the error message contained lib.lora instead of just lora.

I noticed both problems after installing the lora-sx1262 package via Thonny package manager.

Solution

Added require("lora-sync") and require("lora-async") to the manifest files so these dependencies are automatically installed.

To fix the import problem, I added an extra check to prevent this from happening by checking if the error message contains both "no module named", and "lora". Since the check was the same for all imports, I centralized the check in a function.

Testing

Install the package and verify the modem classes are available:

import mip
mip.install("lora-sx126x")
from lora import SX1262, AsyncSX1262  # Should work now

@projectgus
Copy link
Contributor

Hi @brenordv,

Thanks for digging into this and submitting this PR. Not depending on either lora-sync or lora-async is an intentional design choice to save flash size - both these packages are biggish and no one is likely to need both.

We tried to smooth over the usability of this in the installation instructions and by producing a helpful error message if neither package was installed. It seems like your PR also fixes a case where that message wasn't being generated, thanks!

@projectgus projectgus self-assigned this Oct 7, 2025
@projectgus
Copy link
Contributor

@brenordv Thanks for updating the PR.

Maybe I wasn't clear enough in my review: I'm happy to improve the error reporting if both lora-sync and lora-async aren't installed (thanks for that fix).

However, adding both packages as dependencies by default will increase the code size impact of using lora significantly (and almost no one will use both the sync and async APIs on the same board). I'm not inclined to make this change, unless can be convinced it's too difficult to use otherwise.

@brenordv brenordv force-pushed the improving-importing-for-sx-1262 branch from 514f580 to 87747ae Compare November 15, 2025 07:11
Signed-off-by: Breno RdV <breno@raccoon.ninja>
@brenordv brenordv force-pushed the improving-importing-for-sx-1262 branch from 87747ae to 5e59f4a Compare November 15, 2025 07:19
@brenordv
Copy link
Author

@brenordv Thanks for updating the PR.

Maybe I wasn't clear enough in my review: I'm happy to improve the error reporting if both lora-sync and lora-async aren't installed (thanks for that fix).

However, adding both packages as dependencies by default will increase the code size impact of using lora significantly (and almost no one will use both the sync and async APIs on the same board). I'm not inclined to make this change, unless can be convinced it's too difficult to use otherwise.

@projectgus 💯! That makes perfect sense. Come to think of it, I can't see a scenario where you would need both running at the same time. I trimmed it down just to the error message treatment.

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