Add NRF52 support - Githubissues

jacobeva commented 6 months ago

Long time coming eh?

This PR has been designed for and tested on the RAK4630, but hopefully it should support more NRF52 boards too with the correct config.h.

The LoRa driver has been heavily modified to support the SX1262 modem, along with custom pins for SPI communication. To my knowledge before, custom pins could not be used. AFAIK the NRF52 doesn't have hardware SPI, but all the pins are GPIO pins instead, so all SPI is basically software SPI I think.

Preprocessor instructions are heavily used to differentiate between the SX1262 and the older supported modems.

Two way communication using this driver works, messages can be sent in nomadnet, and browsing nodes seems to work in my limited testing. However...

Known issues:

No bluetooth support, need a redesign of the bluetooth driver to support BLE too, which will take longer.
The TCXO preprocessor if statements are currently disabled since I can't figure out how to pull the value from Config.h where I need it. Could you take a look at this Mark?
Same with the MODEM value, I chose to set this in the Makefile instead for the time being, but this is a bad and temporary solution.
RSSI and SNR values are most likely incorrect, but I haven't been able to test them because...
rnprobe doesn't work with the current driver implementation. I don't really know why, it's just quite flaky. I've never managed to get a reply back. Needs more testing.

This driver may also support the SX1280, but I haven't had time to test it yet. I'll give it a shot this weekend if I get my hardware in order.

Please can this PR also be tested on the existing ESP32 boards to ensure I haven't broken anything?

I think that's all for now, I'll keep tweaking this PR according to feedback. Thanks.

u01-rn commented 6 months ago

I've verified I can build the original firmware successfully for the lora32 v2.1. I cannot build it with your changes. Your changes result in ESP32-based microcontrollers including nrf5x code. I cannot suggest any fixes as I am not familiar with rnode-firmware yet.


user@laptop:~/Maint/projects/firm/rnode_firmware_modded/RNode_Firmware$ make firmware-lora32_v21
arduino-cli compile --fqbn esp32:esp32:ttgo-lora32 -e --build-property "build.partitions=no_ota" --build-property "upload.maximum_size=2097152" --build-property "compiler.cpp.extra_flags=\"-DBOARD_MODEL=0x37\""
In file included from /home/user/Maint/projects/firm/rnode_firmware_modded/RNode_Firmware/Utilities.h:19,
                 from /home/user/Maint/projects/firm/rnode_firmware_modded/RNode_Firmware/RNode_Firmware.ino:18:
/home/user/Maint/projects/firm/rnode_firmware_modded/RNode_Firmware/flash_nrf5x.h:28:10: fatal error: common_inc.h: No such file or directory
 #include "common_inc.h"
          ^~~~~~~~~~~~~~
compilation terminated.

Used library Version Path
SPI          2.0.0   /home/user/.arduino15/packages/esp32/hardware/esp32/2.0.11/libraries/SPI

Used platform Version Path
esp32:esp32   2.0.11  /home/user/.arduino15/packages/esp32/hardware/esp32/2.0.11
Error during build: exit status 1
make: *** [Makefile:71: firmware-lora32_v21] Error 1

markqvist commented 6 months ago

Hi Jacob. Thank you so much. This is looking pretty good. I thought you'd have split this out into multiple separate drivers, but you actually did a hybrid driver for all of the SX chips. I'm impressed! :)

We need to maintain compatibility with the old boards before merging this - at least being able to build them, which is not working currently.

Could you let me know what arduino core the nRF52 variant needs to compile? The board manager URL and arduino-cli install core X:Y statement would be great, then we can add a prep-nrf section to the makefile.
The nRF-related flash file comilation errors are easy to fix, just wrap them in a preprocessor macro as noted above
I'd ideally like SPI functionality to stay like it was before, for all of the ESP32 boards, since changing to another SPI mechanism behind the scenes may have consequences for interrupt call paths and such, and suddenly leave me with bugs that will take days to track down. Maybe not, but maybe - and I'd rather avoid that. So if you can drop in the old HW SPI, and just do some namespace mangling there to avoid macros at each SPI-related call, that'd be pretty good.

I'll add more as I go through it, but let's just get those out of the way first.

u01-rn commented 6 months ago

jacobeva's modem driver does compile when just replacing lora.cpp and lora.h from markqvist:master. Some small changes are needed, but even with my rudimentary knowledge of C I was able to do it. Still trying to figure out how to integrate it better (config.h has no effect on the used pins).

Even if you can't get the NRF52 working, this would still do a lot for this project because the SX1276 is becoming increasingly hard to purchase.

I tried to do a port to the Heltec Lora32 V3. The ESP32C3 in it results in the following:

RTC is broken, due to the fact that it is in the SDK
Bluetooth (serial) is NOT supported on the ESP32C3. This means that preprocessor directives will have to be added in console.h. Specifically line 154 where it uses the bluetooth device name for the WiFi SoftAP.
Line 1278 in rnode_firmware is also broken due to the ESP32C3. It doesn't seem to matter if I change HAS_BLUETOOTH to false, this still stops compilation (Serialbt.read)

Even after all this I still couldn't get it to build. I'm going to assume the C3 port is beyond my current skill level.

jacobeva commented 5 months ago

So if you can drop in the old HW SPI, and just do some namespace mangling there to avoid macros at each SPI-related call, that'd be pretty good.

By macros, are you meaning the extern reference I used to spiModem? As in, you'd like to keep the base SPI class and just use that instead? If so, that could be done I think, I'd just have to restructure a bit. If you can confirm this I'll go ahead and do it :)

jacobeva commented 5 months ago

There's lots of errors currently when trying to compile for the genericesp32 variant, I'm working on resolving these now.

Edit: The final error is about spiModem not being defined, I will wait Mark's response to my macro question before going further :)

jacobeva commented 5 months ago

3 Line 1278 in rnode_firmware is also broken due to the ESP32C3. It doesn't seem to matter if I change HAS_BLUETOOTH to false, this still stops compilation (Serialbt.read)

This will be because there was no check in place to see if HAS_BLUETOOTH is true, fixed in 9ef3a0c4b14c78c1f5fb3a20484f37a3e015cbb0

jacobeva commented 5 months ago

I thought you'd have split this out into multiple separate drivers, but you actually did a hybrid driver for all of the SX chips.

There's a question to be had around this design decision. Obviously, if we're going to support multiple modems on one device in the future, this current approach may be rather poor. Would it be worth splitting these drivers into separate files for the future? It may be worth considering to allow for dual modem use.

Edit: Although this said, if the SX1262 and SX1280 drivers are basically the same, this may be unnecessary.

Edit 2: There are differences between the SX1262 and SX1280 drivers. However, they are quite minor and will not take more than a couple of days to sort I imagine.

Edit 3: In fact, I just had an idea. It would probably be possible to add a function to the LoRa class which would allow us to specify the modem which the class should be communicating with, this would allow us to have multiple objects of the LoRa class for different modems, allowing for them to be effectively juggled by the RNode firmware.

jacobeva commented 5 months ago

I accidentally introduced new bugs when faffing around with the spiModem stuff, I've reverted it for now and will redesign the implementation once Mark gets back to me.

markqvist commented 5 months ago

Would it be worth splitting these drivers into separate files for the future? It may be worth considering to allow for dual modem use.

I like it, I think it's compact and efficient as this. We can always split it out rather easily in the future if we need to.

Edit 2: There are differences between the SX1262 and SX1280 drivers. However, they are quite minor and will not take more than a couple of days to sort I imagine.

That's very good news! Awesome to hear!

In fact, I just had an idea. It would probably be possible to add a function to the LoRa class which would allow us to specify the modem which the class should be communicating with, this would allow us to have multiple objects of the LoRa class for different modems, allowing for them to be effectively juggled by the RNode firmware.

That would indeed be very useful, and something that will be necessary in the future. Let's start out with getting the current implementation running and compiling on all platforms first, though.

By macros, are you meaning the extern reference I used to spiModem? As in, you'd like to keep the base SPI class and just use that instead? If so, that could be done I think, I'd just have to restructure a bit. If you can confirm this I'll go ahead and do it :)

Yes, I meant something like just defining a what "SPI" or "spiModem" is, based on the platform, so that we can stick to the normal hardware SPI implementation on the platforms where that has been in use from the beginning, but you can use the software SPI on the nRF if that is the best approach.

I must admit I'm not entirely sure how your current implementation of that works, but to me it looked like this PR swaps all SPI communication to a software-based SPI implementation running over GPIO, instead of the hardware SPI available on (at least) ESP32 and AVR. We need to keep SPI implemented as it currently is on existing platforms, but if you need to use another way on nRF, no problem!

markqvist commented 5 months ago

Ah, and forgot this: Since it seems that the SPI class and the other spiModem you used have exactly the same method call signatures, you can probably get away with just using macros (preprocessors) to create a reference to whatever actual SPI class you want. That way, you don't have to create preprocessor macros for every place in the driver that calls SPI functions.

jacobeva commented 5 months ago

Right. I think 4ca09d65eba494ac0ede9d5ed4bf5992e367e974 should've fixed this. Let me make sure it runs on the nRF52, testing on the old ESP32 boards would be appreciated too. It seems it compiles now.

jacobeva commented 5 months ago

I had to get rid of the BLE stuff as it messes with the flash memory, was causing the nRF port to break. Never expected that, very strange haha. This version should be good to do, BLE may take longer than I thought since it uses Adafruit's LFS, which conflicts with the flash driver I'm using...

markqvist commented 5 months ago

Ok, that is looking great. Things compile now on ESP32 and original RNodes, and I have successful receive and transmit over USB, but Bluetooth completely hangs and fails. I've been taking a quick look at it, but couldn't find any obvious culprits so far. It's a bit strange. I'll be taking a look at it again tomorrow at some point

If you figure something out in that regard in the meantime, or think of anything, please do let me know ;)

Regarding BLE, good call disabling that for now. Let's just get all of this verified and working, and BLE can be added when we have a good base to work from.

jacobeva commented 5 months ago

That should fix it, I ended up commenting part of RNode_Firmware.ino which used SerialBT, since I obviously didn't have support for it :) Now, it should stop trying to use it as HAS_BLUETOOTH is false for the nRF platforms, but should fix your issue.

markqvist commented 5 months ago

Aha! I completely overlooked that. To be fair it was 12.30 pm ;) Will try it out.

markqvist commented 5 months ago

Ok, this is looking good. I'll merge this now, and make a few additions to Config.h to make all the other boards compile properly (just need EEPROM enabled). Please pull those changes into your own repo afterwards.

Also, with make prep-nrf I'm just getting:

arduino-cli core update-index --config-file arduino-cli.yaml
Downloading index: package_index.tar.bz2 downloaded
Downloading index: package_esp32_index.json downloaded
Downloading index: package_rakwireless_index.json downloaded
arduino-cli core install rakwireless:nrf52
Invalid argument passed: Platform 'rakwireless:nrf52' not found
make: *** [Makefile:42: prep-nrf] Error 7

Does this work in your end?

jacobeva commented 5 months ago

Strange issue here. It recognises rakwireless:nrf52 whilst I have it installed, but when uninstalled it says it cannot find it... I have only ever managed to install it through pasting the URL into the board manager box in arduino-ide (GUI) and then searching for and installing it. When installed it says the ID is rakwireless:nrf52 though, very weird! I cannot install it through the CLI at all, any ideas Mark?

jacobeva commented 5 months ago

This command works though: arduino-cli core install rakwireless:nrf52 --additional-urls https://raw.githubusercontent.com/RAKwireless/RAKwireless-Arduino-BSP-Index/main/package_rakwireless_index.json Maybe it's not processing the arduino-cli.yaml properly?

markqvist commented 5 months ago

Thanks. The prep sections in the makefile have been fixed now.

jacobeva commented 5 months ago

That's very funny! It didn't work all this time...

markqvist commented 5 months ago

Apparently, the core install command also needed the specific config file to be specified, otherwise it would not use the downloaded indexes. So this is what fixed it:

arduino-cli core install rakwireless:nrf52 --config-file arduino-cli.yaml

markqvist / RNode_Firmware

Add NRF52 support #55