Initial standalone sx126x driver [WIP] #15795

keever50 · 2025-02-09T12:55:37Z

Summary

This is a driver I am working on for the SX126x (SX1261 and SX1262) LoRa chips. NuttX only had the SX127x drivers before, which are the older variant LoRa chips which are becoming less common.

All functions and definitions are coming directly from the DS SX1261-2 V2.1 datasheet.

There are already existing SX126x drivers out there. For example, Semtech provides one themselves. There is also one written in Rust. This one is not vendor-driven and more free. Ofcourse also specifically made for NuttX in C.

Impact

This chip's output (RF) can communicate with the older variant chips, because it uses the same LoRa modulation. However, there are a few extra features to modulate LoRa beyond what the older variant can do.
For internal compatibility, it is about the same story. It got common features but also got new features.
The drivers in the other hand are not very identical. The chip is being handled differently than the older variant.

This driver allows a lot more IoT boards to be added to NuttX, because most of them using LoRa will use the new SX126x variants.

Testing

Deep testing has yet to be done. Other than there being one test function for development that fires when you open up the device.
This is mainly developed on Linux and a rak11310 board. This rak11310 board is also not yet supported by NuttX because it's main feature is the new LoRa chip. I may add this board to NuttX once it is ready. This might be useful for testing as well.

WIP!

Do not pull this before it is usable. Currently it is not integrated as a driver and purely a preview. I am asking for help or advice on how to take the next steps.
Currently it is missing:

Some less significant hardware features
NuttX driver integration
Filesystem / ioctl
And probably a few other things.

let me know what you think and if you want to help!

nuttxpr · 2025-02-09T12:59:09Z

[Experimental Bot, please feedback here]

Fill In The Commit Message: This PR contains a Commit with an Empty Commit Message. Please fill in the Commit Message with the PR Summary.

This PR, as it stands, does not meet the NuttX requirements. While the idea is promising and addresses a real need, it's currently too incomplete to be considered a proper pull request. Here's why:

Summary: Lacks specific details on what has been implemented. Mentioning "all functions and definitions from the datasheet" is not helpful without specifying which ones are done. The summary should clearly state the current state of the driver (e.g., "Initial implementation of the SX126x driver, including basic initialization and register access").
Impact: While mentioning expanded board support is good, the impact section needs to be more concrete. Currently, it's too general. Since the driver isn't functional, many impacts are unknown. Focus on the current impact of this incomplete PR. For example:
- Impact on user: None yet, as the driver is not usable.
- Impact on build: Minimal, adds source files. (Be specific about which files)
- Impact on hardware: None yet, as the driver is not integrated.
- Impact on documentation: None yet. Documentation will be added upon completion.
- Security: Unknown. Will be assessed upon completion.
- Compatibility: Unknown. Will be addressed upon completion.
Testing: Insufficient. "Deep testing has yet to be done" is unacceptable for a PR. Even for work-in-progress, some basic functionality should be demonstrable and testable. The provided "testing logs" are nonexistent. At the very least, demonstrate register read/write functionality. Even if the driver can't transmit/receive yet, some testing is required.
WIP Status: Marking a PR as WIP is generally discouraged in NuttX (and most open-source projects). It's better to develop on a branch and only submit a PR when it's closer to being ready for review.

Recommendations:

Focus on a minimal viable implementation: Instead of trying to implement everything at once, focus on a core set of functionalities (e.g., initialization, register access, basic transmit/receive). Get that working and tested first.
Provide concrete examples: Show code snippets of the implemented functions, and provide specific register settings used in your test.
Actual testing logs: Include real logs demonstrating the functionality you have implemented.
Break down the work: Create separate issues for individual features (e.g., "Implement LoRa modulation," "Add SPI communication," "Integrate with NuttX driver framework"). This allows for more focused development and review.
Seek feedback earlier: Instead of submitting an incomplete PR, consider opening a discussion on the NuttX mailing list or forums to discuss your approach and get feedback before writing a lot of code.

By following these recommendations, you can create a PR that meets the NuttX requirements and is more likely to be accepted.

acassis · 2025-02-09T13:46:01Z

drivers/wireless/lpwan/sx126x/sx126x.c

+ FAR struct sx126x_dev_s             g_sx126x_devices[SX126X_MAX_DEVICES];
+ static const struct file_operations sx126x_ops =
+ {
+   sx126x_open, sx126x_close, sx126x_read, sx126x_write, 0, 0,


Please put an operation by line, it is easier to stop mistake or missing ops

I agreed. This is very much a placeholder thing. I started off with a template for this. Will do.

keever50 · 2025-02-09T13:59:12Z

I am not entirely sure why all the indents are triple space now. Maybe something happened after copying the file over and it somehow magically auto formatted it to tripple space from doubles. Maybe someone has an idea

keever50 · 2025-02-11T17:49:14Z

hm, i dont think that squashed, or did it

keever50 · 2025-02-11T18:08:25Z

I am not sure if i should keep squashing it with force pushing on every change. Tell me if i am doing something wrong. Learning new things

acassis · 2025-02-11T19:52:38Z

drivers/wireless/lpwan/sx126x/sx126x.c

+      return -EINVAL;
+    }
+
+  printf("Reading\n");


I suggest replacing printf() with syslog info or debug info() macro

Yeah good idea, also just placeholder for now. I am planning on replacing all of these to wlerr, wlinfo, etc. We have one of these already, so might as well. I just partially integrated everything into Nuttx, such as public includes, Kconfig and make files. I am testing everything on a board as described before. Is it a good idea to just add this one into this PR later? That makes things a bit easier.

acassis · 2025-02-11T19:52:52Z

drivers/wireless/lpwan/sx126x/sx126x.c

+      return -EINVAL;
+    }
+
+  printf("Trying to write\n");


acassis · 2025-02-16T14:24:14Z

ping @keever50

keever50 · 2025-02-16T14:30:47Z

pong @acassis

Update:
Still working on it on a different branch to not spam this PR.
Things are fully integrated, added a board for testing and made an app for testing.
Nearly all SX126x features are implemented, except for some ioctl commands.

Just did a ping pong test with two radios and it works perfectly.

Will PR when ready. Most likely this week.

keever50 · 2025-02-16T14:31:30Z

I can drop the updates here for review if you like but it is not fully cleaned up yet.

keever50 · 2025-02-16T15:38:43Z

Let me know if these integrations and ioctl look OK. I would appreciate that. If anything has to be changed lmk

acassis · 2025-02-16T16:17:11Z

Let me know if these integrations and ioctl look OK. I would appreciate that. If anything has to be changed lmk

@keever50 I suggest just separating the board support from the driver. You can still keep everything is in PR, just separate logically the driver from the board

keever50 · 2025-02-16T17:12:22Z

@acassis Is it okay if i create a completely new fresh PR somewhere this week with the clean up and board removed? This makes things a bit easier to manage. Later afterwards i can do a PR for the board and a PR for the example app in the different repo. Ill make sure to link back to this PR. I am still quite new to open source projects.

raiden00pl · 2025-02-18T09:08:18Z

hi, can we make this driver interface compatible with sx127x ? It would be ideally if this driver can work with sx127x_demo example. this will probably require breaking changes in the sx127x API, but in this case it will be justified.

keever50 · 2025-02-18T09:15:55Z

@raiden00pl yes! i was thinking about trying to do this... are we sure we want to break the API? Maybe we can add wrapper macros that point to SX12XX ioctls. That way old apps dont need to be ported.
Both SX126x and SX127x can use the same IOCTL in that case.

keever50 · 2025-02-18T09:19:15Z

There are quite some things in common between these devices. For pretty much all LoRa devices(SF, BW). Maybe it would be nice to have ioctls for common lora devices. Let me know what you think.

raiden00pl · 2025-02-18T09:25:17Z

I think that a portable API is more important than breaking the code for a few users. Breaking changes will be easy to detect and fix (compiler errors), most likely it will be some names change in the user code. The sx127x API was never mature (I say this as the author of this driver), so I think this is a perfect example where a breaking change is justified. But that's just my opinion :)

raiden00pl · 2025-02-18T09:26:59Z

#15828 here is another PR with LORA chip support. Shared ioctl will be beneficial for this driver as well

keever50 · 2025-02-18T09:37:49Z

@raiden00pl good points.

What do we imagine the ioctl commands would look like?

Currently in that PR i see that WLIOC is used to implement specific LoRa features. Personally i think it would be logical to say WLIOC_LORA_xxx instead putting it all on WLIOC that might be shared with other modulations, such as GFSK, OOK etc.
This might make it easier to spot incompatible parameters.

This allows WLIOC_GFSK, WLIOC_OOK etc in the future as well.
I know the sx126x and 127x can benefit from these modulation specifics. They both can do GFSK. 127x can even do OOK.
That solves that problem of awkwardly hiding them away between all the other names.

keever50 · 2025-02-18T20:05:30Z

I will do more testing on this driver and put a test report here this week. Also go over the code for a cleanup.

keever50 · 2025-02-22T12:31:20Z

Fixed some issues after testing the driver using RAK11310 and a demo code.

An echo back and forth test on low spreading factor:
A high spreading factor also works ofcourse

You can see that the RSSI and SNR appear to be incorrect. These values have not been decided on what they should be.
There is still an open issue about creating a common LoRa API. I am blocked for now till that is solved.

However, the driver is fully functional otherwise. There are ofcourse some features not yet implemented, but they are less significant.

The test code is available here. Its not yet PR'd before the driver is pushed. https://pastebin.com/S7iY6Z3m
It does require you to configure a compatible board. Same story. Cant be pushed till driver is pushed.

@acassis

keever50 · 2025-02-22T12:32:50Z

The serial output seems unstable. Missing characters sometimes. I am not entirely sure what is causing that. Might be because my app is using printf and the driver is using logging. Might be causing race conditions.

keever50 · 2025-02-22T12:34:53Z

And obviously, it also works from across the room. Message "Hello" is sent using the app from PC over LoRa.

keever50 · 2025-02-22T13:08:51Z

Actually, @acassis if approved. Can this be pushed? This would significantly help testing it as boards and apps dont have to be removed from the PR every time. The rest of the less significant features, common API and RSSI/SNR can be implemented soon after.

keever50 · 2025-02-23T19:41:20Z

Added a comment to both headers about this driver being currently experimental and breaking changes might happen in the near future.

keever50 · 2025-02-24T10:06:32Z

Signed and changed message. Also includes [experimental] as tag.
It is review ready.

xiaoxiang781216 · 2025-02-24T12:47:52Z

#15828 here is another PR with LORA chip support. Shared ioctl will be beneficial for this driver as well

I suggest to follow the suggestion from @raiden00pl before merging this patch. The unified usersapce interface is very important for any os.

keever50 · 2025-02-24T12:53:42Z

@xiaoxiang781216 Yes thats the idea, however it could help to have this one pushed as experimental driver. So i can push a board and app which can be used for testing.
Currently my board and app are private. The board has to be removed after each merge and people cannot test it the way i can test it.

Later when the common API is decided on, this driver can be updated and the [experimental] tag can be removed. (using a new PR).

However, I'm fine with waiting as well if that works better for NuttX. It might be less efficient.

keever50 · 2025-02-24T12:55:06Z

The common API is discussed here #15856

acassis · 2025-02-24T13:33:48Z

@keever50 please create also the Documentation/ and include the screenshots about the SDR and NSH to let people get more information before using it.

keever50 · 2025-02-24T14:20:59Z

@acassis I can do that.
Is it a good idea if i create a directory called /wireless/lpwan under /documentation/components/drivers/character/ ?
I am not very familiar with this documentation system. Is it possible to manually do this using the github website?

acassis · 2025-02-24T14:26:46Z

@acassis I can do that. Is it a good idea if i create a directory called /wireless/lpwan under /documentation/components/drivers/character/ ? I am not very familiar with this documentation system. Is it possible to manually do this using the github website?

Yes, sounds good!

[Experimental] This adds a driver for the SX126x (SX1261 and SX1262) LoRa chips. All functions and definitions are coming directly from the DS SX1261-2 V2.1 datasheet. Signed-off-by: Kevin Witteveen (MartiniMarter) <[email protected]>

keever50 · 2025-02-24T18:14:20Z

Documentation added

github-actions bot added Area: Drivers Drivers issues Size: XL The size of the change in this PR is very large. Consider breaking down the PR into smaller pieces. labels Feb 9, 2025

acassis reviewed Feb 9, 2025

View reviewed changes

keever50 force-pushed the sx126x_lora_driver branch from 0e88488 to 3fb5506 Compare February 11, 2025 18:07

acassis reviewed Feb 11, 2025

View reviewed changes

keever50 force-pushed the sx126x_lora_driver branch from 3fb5506 to 8936e44 Compare February 16, 2025 15:37

github-actions bot added the Board: arm label Feb 16, 2025

raiden00pl mentioned this pull request Feb 18, 2025

drivers/wireless: Add support for the RN903 and RN2483 LoRa modules #15828

Draft

raiden00pl mentioned this pull request Feb 18, 2025

[FEATURE] Common interface for LORA chips #15856

Open

1 task

keever50 force-pushed the sx126x_lora_driver branch from 8936e44 to e3485a0 Compare February 18, 2025 20:03

github-actions bot removed the Board: arm label Feb 18, 2025

keever50 force-pushed the sx126x_lora_driver branch 2 times, most recently from c1e8ddb to 66d63cc Compare February 22, 2025 12:25

keever50 marked this pull request as ready for review February 22, 2025 13:26

keever50 force-pushed the sx126x_lora_driver branch 2 times, most recently from d70d248 to ab612b6 Compare February 23, 2025 19:40

keever50 force-pushed the sx126x_lora_driver branch from ab612b6 to 17f7213 Compare February 24, 2025 10:04

jerpelea approved these changes Feb 24, 2025

View reviewed changes

tmedicci approved these changes Feb 24, 2025

View reviewed changes

drivers/wireless/lpwan/sx126x:

c3a0b01

[Experimental] This adds a driver for the SX126x (SX1261 and SX1262) LoRa chips. All functions and definitions are coming directly from the DS SX1261-2 V2.1 datasheet. Signed-off-by: Kevin Witteveen (MartiniMarter) <[email protected]>

keever50 force-pushed the sx126x_lora_driver branch from 17f7213 to c3a0b01 Compare February 24, 2025 18:13

github-actions bot added the Area: Documentation Improvements or additions to documentation label Feb 24, 2025

Initial standalone sx126x driver [WIP] #15795

Are you sure you want to change the base?

Initial standalone sx126x driver [WIP] #15795

Conversation

keever50 commented Feb 9, 2025

Summary

Impact

Testing

WIP!

nuttxpr commented Feb 9, 2025

acassis Feb 9, 2025

Choose a reason for hiding this comment

keever50 Feb 9, 2025

Choose a reason for hiding this comment

keever50 commented Feb 9, 2025

keever50 commented Feb 11, 2025

keever50 commented Feb 11, 2025

acassis Feb 11, 2025

Choose a reason for hiding this comment

keever50 Feb 11, 2025 • edited Loading

Choose a reason for hiding this comment

acassis Feb 11, 2025

Choose a reason for hiding this comment

acassis commented Feb 16, 2025

keever50 commented Feb 16, 2025

keever50 commented Feb 16, 2025

keever50 commented Feb 16, 2025

acassis commented Feb 16, 2025

keever50 commented Feb 16, 2025

raiden00pl commented Feb 18, 2025

keever50 commented Feb 18, 2025

keever50 commented Feb 18, 2025

raiden00pl commented Feb 18, 2025

raiden00pl commented Feb 18, 2025

keever50 commented Feb 18, 2025

keever50 commented Feb 18, 2025 • edited Loading

keever50 commented Feb 22, 2025

keever50 commented Feb 22, 2025

keever50 commented Feb 22, 2025

keever50 commented Feb 22, 2025

keever50 commented Feb 23, 2025

keever50 commented Feb 24, 2025

xiaoxiang781216 commented Feb 24, 2025 • edited Loading

keever50 commented Feb 24, 2025

keever50 commented Feb 24, 2025

acassis commented Feb 24, 2025

keever50 commented Feb 24, 2025

acassis commented Feb 24, 2025

keever50 commented Feb 24, 2025

keever50 Feb 11, 2025 •

edited

Loading

keever50 commented Feb 18, 2025 •

edited

Loading

xiaoxiang781216 commented Feb 24, 2025 •

edited

Loading