Lora-net / LoRaMac-node

Reference implementation and documentation of a LoRa network node.
Other
1.88k stars 1.09k forks source link

Missing function in the SX126x driver for switching the RX/TX path #917

Closed ZaneL closed 4 years ago

ZaneL commented 4 years ago

Hello,

I just bought one of these modules: https://www.mouser.com/ProductDetail/RF-Solutions/LAMBDA62C-9D?qs=17u8i%2FzlE89eJC9fRV1x7A%3D%3D

...and in the datasheet it shows that there is a separate pin to enable the TX path and separate pin to enable the RX path. https://www.mouser.com/datasheet/2/975/1551175771DS-LAMBDA62-2-1624682.pdf

I think to get this to work I'm going to have to modify the driver -- specifically the SX126xSetTx() and SX126xSetRx() functions where I'd add a couple lines in each function to set the TX pin high and the RX pin low for example.

It would be nice if the driver supported this by default. It looks like Semtech recommends hardware designers connect the TX/RX path enable to DIO2 (and then you'd call SX126xSetDio2AsRfSwitchCtrl()), but this manufacturer just ignored that recommendation?

andysan commented 4 years ago

We have the same issue in the LoRaMAC integration in Zephyr. My proposed solution is here: https://github.com/zephyrproject-rtos/zephyr/pull/26611 & https://github.com/zephyrproject-rtos/loramac-node/pull/5

A solution that would work well for us would be to do the following:

There is also this issue that affects the RF switch (only affects testing): https://github.com/Lora-net/LoRaMac-node/issues/926

Mani-Sadhasivam commented 4 years ago

@mluis1 Can you please comment on this? We are really trying to avoid hacks in our Zephyr fork and would love to see a generic solution in upstream loramac-node repo which could work for other projects as well.

mluis1 commented 4 years ago

I think that it is a good proposition. If you look at the LR1110 radio driver something similar to this proposition is already present.

Would the below patch suit your needs

sx126x_mcu_rf_switch_ctrl.patch

Click to expand!

```c diff --git a/src/boards/NucleoL073/sx1261mbxbas-board.c b/src/boards/NucleoL073/sx1261mbxbas-board.c index d11db3ab..0ee534bd 100644 --- a/src/boards/NucleoL073/sx1261mbxbas-board.c +++ b/src/boards/NucleoL073/sx1261mbxbas-board.c @@ -28,6 +28,11 @@ #include "radio.h" #include "sx126x-board.h" +/*! + * \brief Holds the internal operating mode of the radio + */ +static RadioOperatingModes_t OperatingMode; + /*! * Antenna switch GPIO pins objects */ @@ -80,6 +85,39 @@ uint32_t SX126xGetBoardTcxoWakeupTime( void ) return BOARD_TCXO_WAKEUP_TIME; } +void SX126xIoRfSwitchInit( void ) +{ + SX126xSetDio2AsRfSwitchCtrl( true ); +} + +RadioOperatingModes_t SX126xGetOperatingMode( void ) +{ + return OperatingMode; +} + +void SX126xSetOperatingMode( RadioOperatingModes_t mode ) +{ + OperatingMode = mode; +#if defined( USE_RADIO_DEBUG ) + switch( mode ) + { + case MODE_TX: + SX126xDbgPinTxWrite( 1 ); + SX126xDbgPinRxWrite( 0 ); + break; + case MODE_RX: + case MODE_RX_DC: + SX126xDbgPinTxWrite( 0 ); + SX126xDbgPinRxWrite( 1 ); + break; + default: + SX126xDbgPinTxWrite( 0 ); + SX126xDbgPinRxWrite( 0 ); + break; + } +#endif +} + void SX126xReset( void ) { DelayMs( 10 ); diff --git a/src/boards/NucleoL073/sx1262mbxcas-board.c b/src/boards/NucleoL073/sx1262mbxcas-board.c index 29e2d52a..5e82ab08 100644 --- a/src/boards/NucleoL073/sx1262mbxcas-board.c +++ b/src/boards/NucleoL073/sx1262mbxcas-board.c @@ -28,6 +28,11 @@ #include "radio.h" #include "sx126x-board.h" +/*! + * \brief Holds the internal operating mode of the radio + */ +static RadioOperatingModes_t OperatingMode; + /*! * Antenna switch GPIO pins objects */ @@ -80,6 +85,39 @@ uint32_t SX126xGetBoardTcxoWakeupTime( void ) return BOARD_TCXO_WAKEUP_TIME; } +void SX126xIoRfSwitchInit( void ) +{ + SX126xSetDio2AsRfSwitchCtrl( true ); +} + +RadioOperatingModes_t SX126xGetOperatingMode( void ) +{ + return OperatingMode; +} + +void SX126xSetOperatingMode( RadioOperatingModes_t mode ) +{ + OperatingMode = mode; +#if defined( USE_RADIO_DEBUG ) + switch( mode ) + { + case MODE_TX: + SX126xDbgPinTxWrite( 1 ); + SX126xDbgPinRxWrite( 0 ); + break; + case MODE_RX: + case MODE_RX_DC: + SX126xDbgPinTxWrite( 0 ); + SX126xDbgPinRxWrite( 1 ); + break; + default: + SX126xDbgPinTxWrite( 0 ); + SX126xDbgPinRxWrite( 0 ); + break; + } +#endif +} + void SX126xReset( void ) { DelayMs( 10 ); diff --git a/src/boards/NucleoL073/sx1262mbxdas-board.c b/src/boards/NucleoL073/sx1262mbxdas-board.c index f665f725..fa6d977b 100644 --- a/src/boards/NucleoL073/sx1262mbxdas-board.c +++ b/src/boards/NucleoL073/sx1262mbxdas-board.c @@ -31,6 +31,11 @@ #include "radio.h" #include "sx126x-board.h" +/*! + * \brief Holds the internal operating mode of the radio + */ +static RadioOperatingModes_t OperatingMode; + /*! * Antenna switch GPIO pins objects */ @@ -87,6 +92,39 @@ uint32_t SX126xGetBoardTcxoWakeupTime( void ) return BOARD_TCXO_WAKEUP_TIME; } +void SX126xIoRfSwitchInit( void ) +{ + SX126xSetDio2AsRfSwitchCtrl( true ); +} + +RadioOperatingModes_t SX126xGetOperatingMode( void ) +{ + return OperatingMode; +} + +void SX126xSetOperatingMode( RadioOperatingModes_t mode ) +{ + OperatingMode = mode; +#if defined( USE_RADIO_DEBUG ) + switch( mode ) + { + case MODE_TX: + SX126xDbgPinTxWrite( 1 ); + SX126xDbgPinRxWrite( 0 ); + break; + case MODE_RX: + case MODE_RX_DC: + SX126xDbgPinTxWrite( 0 ); + SX126xDbgPinRxWrite( 1 ); + break; + default: + SX126xDbgPinTxWrite( 0 ); + SX126xDbgPinRxWrite( 0 ); + break; + } +#endif +} + void SX126xReset( void ) { DelayMs( 10 ); diff --git a/src/boards/NucleoL152/sx1261mbxbas-board.c b/src/boards/NucleoL152/sx1261mbxbas-board.c index d11db3ab..0ee534bd 100644 --- a/src/boards/NucleoL152/sx1261mbxbas-board.c +++ b/src/boards/NucleoL152/sx1261mbxbas-board.c @@ -28,6 +28,11 @@ #include "radio.h" #include "sx126x-board.h" +/*! + * \brief Holds the internal operating mode of the radio + */ +static RadioOperatingModes_t OperatingMode; + /*! * Antenna switch GPIO pins objects */ @@ -80,6 +85,39 @@ uint32_t SX126xGetBoardTcxoWakeupTime( void ) return BOARD_TCXO_WAKEUP_TIME; } +void SX126xIoRfSwitchInit( void ) +{ + SX126xSetDio2AsRfSwitchCtrl( true ); +} + +RadioOperatingModes_t SX126xGetOperatingMode( void ) +{ + return OperatingMode; +} + +void SX126xSetOperatingMode( RadioOperatingModes_t mode ) +{ + OperatingMode = mode; +#if defined( USE_RADIO_DEBUG ) + switch( mode ) + { + case MODE_TX: + SX126xDbgPinTxWrite( 1 ); + SX126xDbgPinRxWrite( 0 ); + break; + case MODE_RX: + case MODE_RX_DC: + SX126xDbgPinTxWrite( 0 ); + SX126xDbgPinRxWrite( 1 ); + break; + default: + SX126xDbgPinTxWrite( 0 ); + SX126xDbgPinRxWrite( 0 ); + break; + } +#endif +} + void SX126xReset( void ) { DelayMs( 10 ); diff --git a/src/boards/NucleoL152/sx1262mbxcas-board.c b/src/boards/NucleoL152/sx1262mbxcas-board.c index 29e2d52a..5e82ab08 100644 --- a/src/boards/NucleoL152/sx1262mbxcas-board.c +++ b/src/boards/NucleoL152/sx1262mbxcas-board.c @@ -28,6 +28,11 @@ #include "radio.h" #include "sx126x-board.h" +/*! + * \brief Holds the internal operating mode of the radio + */ +static RadioOperatingModes_t OperatingMode; + /*! * Antenna switch GPIO pins objects */ @@ -80,6 +85,39 @@ uint32_t SX126xGetBoardTcxoWakeupTime( void ) return BOARD_TCXO_WAKEUP_TIME; } +void SX126xIoRfSwitchInit( void ) +{ + SX126xSetDio2AsRfSwitchCtrl( true ); +} + +RadioOperatingModes_t SX126xGetOperatingMode( void ) +{ + return OperatingMode; +} + +void SX126xSetOperatingMode( RadioOperatingModes_t mode ) +{ + OperatingMode = mode; +#if defined( USE_RADIO_DEBUG ) + switch( mode ) + { + case MODE_TX: + SX126xDbgPinTxWrite( 1 ); + SX126xDbgPinRxWrite( 0 ); + break; + case MODE_RX: + case MODE_RX_DC: + SX126xDbgPinTxWrite( 0 ); + SX126xDbgPinRxWrite( 1 ); + break; + default: + SX126xDbgPinTxWrite( 0 ); + SX126xDbgPinRxWrite( 0 ); + break; + } +#endif +} + void SX126xReset( void ) { DelayMs( 10 ); diff --git a/src/boards/NucleoL152/sx1262mbxdas-board.c b/src/boards/NucleoL152/sx1262mbxdas-board.c index f665f725..fa6d977b 100644 --- a/src/boards/NucleoL152/sx1262mbxdas-board.c +++ b/src/boards/NucleoL152/sx1262mbxdas-board.c @@ -31,6 +31,11 @@ #include "radio.h" #include "sx126x-board.h" +/*! + * \brief Holds the internal operating mode of the radio + */ +static RadioOperatingModes_t OperatingMode; + /*! * Antenna switch GPIO pins objects */ @@ -87,6 +92,39 @@ uint32_t SX126xGetBoardTcxoWakeupTime( void ) return BOARD_TCXO_WAKEUP_TIME; } +void SX126xIoRfSwitchInit( void ) +{ + SX126xSetDio2AsRfSwitchCtrl( true ); +} + +RadioOperatingModes_t SX126xGetOperatingMode( void ) +{ + return OperatingMode; +} + +void SX126xSetOperatingMode( RadioOperatingModes_t mode ) +{ + OperatingMode = mode; +#if defined( USE_RADIO_DEBUG ) + switch( mode ) + { + case MODE_TX: + SX126xDbgPinTxWrite( 1 ); + SX126xDbgPinRxWrite( 0 ); + break; + case MODE_RX: + case MODE_RX_DC: + SX126xDbgPinTxWrite( 0 ); + SX126xDbgPinRxWrite( 1 ); + break; + default: + SX126xDbgPinTxWrite( 0 ); + SX126xDbgPinRxWrite( 0 ); + break; + } +#endif +} + void SX126xReset( void ) { DelayMs( 10 ); diff --git a/src/boards/NucleoL476/sx1261mbxbas-board.c b/src/boards/NucleoL476/sx1261mbxbas-board.c index d11db3ab..0ee534bd 100644 --- a/src/boards/NucleoL476/sx1261mbxbas-board.c +++ b/src/boards/NucleoL476/sx1261mbxbas-board.c @@ -28,6 +28,11 @@ #include "radio.h" #include "sx126x-board.h" +/*! + * \brief Holds the internal operating mode of the radio + */ +static RadioOperatingModes_t OperatingMode; + /*! * Antenna switch GPIO pins objects */ @@ -80,6 +85,39 @@ uint32_t SX126xGetBoardTcxoWakeupTime( void ) return BOARD_TCXO_WAKEUP_TIME; } +void SX126xIoRfSwitchInit( void ) +{ + SX126xSetDio2AsRfSwitchCtrl( true ); +} + +RadioOperatingModes_t SX126xGetOperatingMode( void ) +{ + return OperatingMode; +} + +void SX126xSetOperatingMode( RadioOperatingModes_t mode ) +{ + OperatingMode = mode; +#if defined( USE_RADIO_DEBUG ) + switch( mode ) + { + case MODE_TX: + SX126xDbgPinTxWrite( 1 ); + SX126xDbgPinRxWrite( 0 ); + break; + case MODE_RX: + case MODE_RX_DC: + SX126xDbgPinTxWrite( 0 ); + SX126xDbgPinRxWrite( 1 ); + break; + default: + SX126xDbgPinTxWrite( 0 ); + SX126xDbgPinRxWrite( 0 ); + break; + } +#endif +} + void SX126xReset( void ) { DelayMs( 10 ); diff --git a/src/boards/NucleoL476/sx1262mbxcas-board.c b/src/boards/NucleoL476/sx1262mbxcas-board.c index 29e2d52a..5e82ab08 100644 --- a/src/boards/NucleoL476/sx1262mbxcas-board.c +++ b/src/boards/NucleoL476/sx1262mbxcas-board.c @@ -28,6 +28,11 @@ #include "radio.h" #include "sx126x-board.h" +/*! + * \brief Holds the internal operating mode of the radio + */ +static RadioOperatingModes_t OperatingMode; + /*! * Antenna switch GPIO pins objects */ @@ -80,6 +85,39 @@ uint32_t SX126xGetBoardTcxoWakeupTime( void ) return BOARD_TCXO_WAKEUP_TIME; } +void SX126xIoRfSwitchInit( void ) +{ + SX126xSetDio2AsRfSwitchCtrl( true ); +} + +RadioOperatingModes_t SX126xGetOperatingMode( void ) +{ + return OperatingMode; +} + +void SX126xSetOperatingMode( RadioOperatingModes_t mode ) +{ + OperatingMode = mode; +#if defined( USE_RADIO_DEBUG ) + switch( mode ) + { + case MODE_TX: + SX126xDbgPinTxWrite( 1 ); + SX126xDbgPinRxWrite( 0 ); + break; + case MODE_RX: + case MODE_RX_DC: + SX126xDbgPinTxWrite( 0 ); + SX126xDbgPinRxWrite( 1 ); + break; + default: + SX126xDbgPinTxWrite( 0 ); + SX126xDbgPinRxWrite( 0 ); + break; + } +#endif +} + void SX126xReset( void ) { DelayMs( 10 ); diff --git a/src/boards/NucleoL476/sx1262mbxdas-board.c b/src/boards/NucleoL476/sx1262mbxdas-board.c index f665f725..fa6d977b 100644 --- a/src/boards/NucleoL476/sx1262mbxdas-board.c +++ b/src/boards/NucleoL476/sx1262mbxdas-board.c @@ -31,6 +31,11 @@ #include "radio.h" #include "sx126x-board.h" +/*! + * \brief Holds the internal operating mode of the radio + */ +static RadioOperatingModes_t OperatingMode; + /*! * Antenna switch GPIO pins objects */ @@ -87,6 +92,39 @@ uint32_t SX126xGetBoardTcxoWakeupTime( void ) return BOARD_TCXO_WAKEUP_TIME; } +void SX126xIoRfSwitchInit( void ) +{ + SX126xSetDio2AsRfSwitchCtrl( true ); +} + +RadioOperatingModes_t SX126xGetOperatingMode( void ) +{ + return OperatingMode; +} + +void SX126xSetOperatingMode( RadioOperatingModes_t mode ) +{ + OperatingMode = mode; +#if defined( USE_RADIO_DEBUG ) + switch( mode ) + { + case MODE_TX: + SX126xDbgPinTxWrite( 1 ); + SX126xDbgPinRxWrite( 0 ); + break; + case MODE_RX: + case MODE_RX_DC: + SX126xDbgPinTxWrite( 0 ); + SX126xDbgPinRxWrite( 1 ); + break; + default: + SX126xDbgPinTxWrite( 0 ); + SX126xDbgPinRxWrite( 0 ); + break; + } +#endif +} + void SX126xReset( void ) { DelayMs( 10 ); diff --git a/src/boards/sx126x-board.h b/src/boards/sx126x-board.h index 1589106e..a6cdcf32 100644 --- a/src/boards/sx126x-board.h +++ b/src/boards/sx126x-board.h @@ -56,6 +56,11 @@ void SX126xIoDeInit( void ); */ void SX126xIoTcxoInit( void ); +/*! + * \brief Initializes RF switch control pins. + */ +void SX126xIoRfSwitchInit( void ); + /*! * \brief Initializes the radio debug pins. */ @@ -154,6 +159,23 @@ bool SX126xCheckRfFrequency( uint32_t frequency ); */ uint32_t SX126xGetBoardTcxoWakeupTime( void ); +/*! + * \brief Gets the current Radio OperationMode variable + * + * \retval RadioOperatingModes_t last operating mode + */ +RadioOperatingModes_t SX126xGetOperatingMode( void ); + +/*! + * \brief Sets/Updates the current Radio OperationMode variable. + * + * \remark WARNING: This function is only required to reflect the current radio + * operating mode when processing interrupts. + * + * \param [in] mode New operating mode + */ +void SX126xSetOperatingMode( RadioOperatingModes_t mode ); + /*! * \brief Writes new Tx debug pin state * diff --git a/src/radio/sx126x/sx126x.c b/src/radio/sx126x/sx126x.c index 5f99056e..7b1c995b 100644 --- a/src/radio/sx126x/sx126x.c +++ b/src/radio/sx126x/sx126x.c @@ -37,11 +37,6 @@ typedef struct uint8_t Value; //!< The value of the register }RadioRegisters_t; -/*! - * \brief Holds the internal operating mode of the radio - */ -static RadioOperatingModes_t OperatingMode; - /*! * \brief Stores the current packet type set in the radio */ @@ -98,36 +93,10 @@ void SX126xInit( DioIrqHandler dioIrq ) // Initialize TCXO control SX126xIoTcxoInit( ); - SX126xSetDio2AsRfSwitchCtrl( true ); - SX126xSetOperatingMode( MODE_STDBY_RC ); -} - -RadioOperatingModes_t SX126xGetOperatingMode( void ) -{ - return OperatingMode; -} + // Initialize RF switch control + SX126xIoRfSwitchInit( ); -void SX126xSetOperatingMode( RadioOperatingModes_t mode ) -{ - OperatingMode = mode; -#if defined( USE_RADIO_DEBUG ) - switch( mode ) - { - case MODE_TX: - SX126xDbgPinTxWrite( 1 ); - SX126xDbgPinRxWrite( 0 ); - break; - case MODE_RX: - case MODE_RX_DC: - SX126xDbgPinTxWrite( 0 ); - SX126xDbgPinRxWrite( 1 ); - break; - default: - SX126xDbgPinTxWrite( 0 ); - SX126xDbgPinRxWrite( 0 ); - break; - } -#endif + SX126xSetOperatingMode( MODE_STDBY_RC ); } void SX126xCheckDeviceReady( void ) diff --git a/src/radio/sx126x/sx126x.h b/src/radio/sx126x/sx126x.h index 699c3408..3bc7bac3 100644 --- a/src/radio/sx126x/sx126x.h +++ b/src/radio/sx126x/sx126x.h @@ -735,23 +735,6 @@ typedef struct */ void SX126xInit( DioIrqHandler dioIrq ); -/*! - * \brief Gets the current Radio OperationMode variable - * - * \retval RadioOperatingModes_t last operating mode - */ -RadioOperatingModes_t SX126xGetOperatingMode( void ); - -/*! - * \brief Sets/Updates the current Radio OperationMode variable. - * - * \remark WARNING: This function is only required to reflect the current radio - * operating mode when processing interrupts. - * - * \param [in] mode New operating mode - */ -void SX126xSetOperatingMode( RadioOperatingModes_t mode ); - /*! * \brief Wakeup the radio if it is in Sleep mode and check that Busy is low */ ```

andysan commented 4 years ago

That change should work for us, that's basically what I have been doing using ifdefs on my branch. If you merge this, you probably want to remove SX126xDbgPinTxWrite and SX126xDbgPinRxWrite from src/boards/sx126x-board.h as well since they won't be used by the HAL.

mluis1 commented 4 years ago

Thanks for the feedback. I'll apply your recommendations.