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

Add support for RAK3172 module #13

Closed
wants to merge 1 commit into from

Conversation

Miceuz
Copy link
Contributor

@Miceuz Miceuz commented Feb 13, 2023

assume its board description will have GENERIC_WLE5CCUX as a board. Fix #7

@Oliv4945
Copy link

Hello @Miceuz , thanks for this PR but I wonder if another config related to the FR switch should be changed? As per my understanding of this the RAK3172 has an antenna switch with RX/TX_HP but no TX_LP path, so

// Supported TX modes (LP/HP or both)
#if !defined(LORAWAN_TX_CONFIG)
  #define LORAWAN_TX_CONFIG RBI_CONF_RFO_LP_HP
#endif

Should be changed to

// Supported TX modes (LP/HP or both)
#if !defined(LORAWAN_TX_CONFIG)
  #if defined(ARDUINO_NUCLEO_WL55JC1)
    #define LORAWAN_TX_CONFIG RBI_CONF_RFO_LP_HP
  #elif defined(ARDUINO_GENERIC_WLE5CCUX)
    #define LORAWAN_TX_CONFIG RBI_CONF_RFO_HP
  #endif
#endif

This is also probably why you set the same value for LORAWAN_RFSWITCH_RFO_LP_VALUES and LORAWAN_RFSWITCH_RFO_HP_VALUES.

Doing this change actually makes the library work for me

@Miceuz
Copy link
Contributor Author

Miceuz commented Mar 15, 2023

@Oliv4945 I don't mind adding this change, but I am not sure about the naming.

Now we are sort of declaring ARDUINO_GENERIC_WLE5CCUX to be RAK3172 - do we want that? This applies both to my previous changes and to the change you are proposing.

@fpistm fpistm added the wontfix This will not be worked on label Mar 24, 2023
@fpistm
Copy link
Member

fpistm commented Mar 24, 2023

Hi @Miceuz
Thanks for this contribution but as stated:

  • in RAK3172 "WLE5CCU6" #7 the RAK3172 should be added as a variants with the specific RF definitions
  • by you:

Now we are sort of declaring ARDUINO_GENERIC_WLE5CCUX to be RAK3172 - do we want that?
answer is no else other generic will probably not work. Moreover, we planned to add API to specify the RFSwith pins which will allow to use generic without specific RF definitions.

So I close this PR as it will not be merged.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
wontfix This will not be worked on
Projects
None yet
Development

Successfully merging this pull request may close these issues.

RAK3172 "WLE5CCU6"
3 participants