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

Semtech sx126x Lora radio driver #344

Merged
merged 1 commit into from
Jan 15, 2022
Merged

Conversation

ofauchon
Copy link
Contributor

This is an early implementation of driver for Semtech sx126x radios .

This driver will support :

  • Standalone, SX126x modules, connected to MCUs through regular SPI buses, and NSS/RST/BUSY GPIOS
  • Sx126x radios embedded in SoC (ex : in STM32WL series )

When using sx126x in SoCs, you may not have 'regular' SPI BUS and GPIOs (but specific registers instead).
It may require specific initialization sequences too

The driver uses build tag to split the driver code, and low lever stuff:

  • sx126x.go : The driver itself without low level / hardware code
  • spi_stm32wl.go : Specific code for SPI/NSS/Busy/RST/initialization when using embedded SX126X from STM32WLxx SoC
  • spi_generic.go : Wrapper for standard SPI and GPIOs access (When using 'regular' sx126x modules)

Although the driver may (almost) work, it's WIP.

The PR is a Draft to get feedback on the implementation.

@ofauchon ofauchon marked this pull request as draft November 26, 2021 22:27
@ofauchon
Copy link
Contributor Author

Hi.
Could you please tell me if the implementation (with build tags) is the correct way to deal with
the differents hardware scenarios ?

Thx

Olivier

@deadprogram
Copy link
Member

I have a couple of Arduino MKR 1300 boards on hand. They say they have this for the onboard LoraWAN module: https://content.arduino.cc/assets/mkrwan1310-murata_lora_module-type_abz.pdf

The datasheet says this module uses a SX1276 chip. Will this driver work with that?

@ofauchon
Copy link
Contributor Author

ofauchon commented Dec 6, 2021

Hi @deadprogram,

SX126X and SX127X have differents SPI protocols. So each one have its own driver.

I also wrote a driver for SX127x too : https://github.com/ofauchon/tinygo-drivers/tree/lora-sx127x/lora/sx127x

But at the moment, I'm giving focus on SX126x (and sx127x driver developpement is paused)

If you want to give a try, have a look to the example :

https://github.com/ofauchon/tinygo-drivers/blob/lora-sx127x/examples/lora/sx127x/pingpong.go

Or at least you could try to detect the SX127x radio module on the SPI with the following piece of code:

https://github.com/ofauchon/tinygo-drivers/blob/515610326985ddc25c32b06c6c74655413c3cafa/lora/sx127x/sx127x.go#L126-L130

NB: sx126x is under active developement, and many improvements will have to be backported to SX127x:
(Use DIO interrupts instead of IRQ Status register polling, Radio Event channnel refactoring ...)

Feel free ton contact me on slack if you need more informations.

Olivier

@ofauchon ofauchon force-pushed the lora-sx126x branch 3 times, most recently from d911e83 to c4d248c Compare December 12, 2021 22:43
@ofauchon ofauchon force-pushed the lora-sx126x branch 4 times, most recently from 05b38db to 07c400b Compare December 21, 2021 00:29
@ofauchon ofauchon force-pushed the lora-sx126x branch 6 times, most recently from c40f0e3 to 05aff68 Compare January 1, 2022 14:14
@ofauchon ofauchon marked this pull request as ready for review January 1, 2022 14:16
@ofauchon ofauchon changed the title Semtech sx126x Lora radio driver WIP/DRAFT Semtech sx126x Lora radio driver Jan 1, 2022
@ofauchon
Copy link
Contributor Author

ofauchon commented Jan 1, 2022

Here is a first version of the SX126x driver.
I could test it with Generic Node Sensor Edition and nucleo-wl55jc devices.

sx126x/sx126x.go Outdated Show resolved Hide resolved
sx126x/spi_stm32wl.go Outdated Show resolved Hide resolved
sx126x/spi_stm32wl.go Outdated Show resolved Hide resolved
sx126x/spi_stm32wl.go Outdated Show resolved Hide resolved
sx126x/sx126x.go Outdated
d.SpiTx(uint8(addr & 0x00FF))
d.SpiTx(uint8(0x00)) // This byte is for status (unused yet)

var ret []uint8
Copy link
Member

Choose a reason for hiding this comment

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

You can define this as var ret [size]unit8 to avoid heap allocations, and then instead of the loop use something like this:

d.spi.Tx(nil, ret[:])

Copy link
Contributor Author

@ofauchon ofauchon Jan 5, 2022

Choose a reason for hiding this comment

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

var ret [size]uint8 fails with 

... fails with :

array length size (variable of type uint16) must be constant

I could also declare it as a static array with some max size (eg: ret[MAX_READ_SIZE]uint8), but would that make sense ?

Copy link
Member

Choose a reason for hiding this comment

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

I could also declare it as a static array with some max size (eg: ret[MAX_READ_SIZE]uint8), but would that make sense ?

Yes that would be better. The idea is to avoid all of the heap allocations by reusing an array as a slice hence avoiding use of make when not really needed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks @deadprogram

Do you think I could use such commands:

tinygo build -print-allocs=. -target gnse -o test.hex  2>&1 | grep heap  | grep sx126x
/home/olivier/go/src/tinygo.org/x/drivers/sx126x/sx126x.go:293:14: object allocated on the heap: escapes at line 294
/home/olivier/go/src/tinygo.org/x/drivers/sx126x/spi_stm32wl.go:20:22: object allocated on the heap: escapes at line 20
/home/olivier/go/src/tinygo.org/x/drivers/sx126x/spi_stm32wl.go:16:2: object allocated on the heap: escapes at line 26

... to improve my code ?
Because it doesn't report heap allocation on the line you spotted earlier.

253	var ret []uint8

Can this be an potential issue at runtime ?

Thanks.

Copy link
Member

Choose a reason for hiding this comment

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

That is a question best answered by @aykevl

Copy link
Contributor Author

@ofauchon ofauchon Jan 12, 2022

Choose a reason for hiding this comment

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

At the end, I defined a spiBuffer[SPI_BUFFER_SIZE] with SPI_BUFFER_SIZE=256 (in the device struct) .
256 is the max allowed size for reading datas from the radio.

Please tell me if it's good or not (My mind is not yet very clear on stack/heap usage)

Thx

sx126x/sx126x.go Outdated Show resolved Hide resolved
sx126x/sx126x.go Outdated Show resolved Hide resolved
@deadprogram
Copy link
Member

I made some comments, you can extrapolate to other parts of the code that use the same patterns.

Also, please note the merge conflict.

sx126x/sx126x.go Outdated Show resolved Hide resolved
@ofauchon ofauchon force-pushed the lora-sx126x branch 3 times, most recently from 611bd1f to ff04ca8 Compare January 12, 2022 23:09
sx126x/sx126x.go Outdated Show resolved Hide resolved
	This first version of the driver has been tested with STM32WL SoC,
	which embeddeds SX1262 radio on the same die.
@deadprogram
Copy link
Member

Thanks for making all the changes @ofauchon and for such a powerful contribution! Now merging.

@deadprogram deadprogram merged commit b6c750c into tinygo-org:dev Jan 15, 2022
@ofauchon ofauchon deleted the lora-sx126x branch January 15, 2022 20:31
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