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 RAK3172 module #2005

Merged
merged 1 commit into from
Apr 17, 2023
Merged

Add RAK3172 module #2005

merged 1 commit into from
Apr 17, 2023

Conversation

Oliv4945
Copy link
Contributor

Summary

This PR implements RAK3172 module bases on STM32WLE5CCU

As discussed the LoRaWAN repo (stm32duino/STM32LoRaWAN#13) it is cleaner to add it here.
This is still a draft as I did not update the main Readme because of a question: I created the section LoRa modules , but is it correct? If not, should I place it in the LoRa boards instead?

Validation

  • LoRaWAN works on the module. I did not test the other peripherals like SPI or I2C

@fpistm
Copy link
Member

fpistm commented Apr 13, 2023

Hi @Oliv4945
Thanks for this contribution.
Please add it to the LoRa section to avoid adding new one. I will review it tomorrow.

@Oliv4945
Copy link
Contributor Author

Thanks @fpistm, I moved it.
I put a green heart as the "generic WLE5CC" but I saw that you put yellow on the Seeedstudio's E5 module, so please tell me your policy ;)

@fpistm fpistm added this to In progress in STM32 core based on ST HAL via automation Apr 14, 2023
@fpistm fpistm added this to the 2.6.0 milestone Apr 14, 2023
@fpistm
Copy link
Member

fpistm commented Apr 14, 2023

Well while the new release including the board is not released I put it in yellow and moved to green when officially released.

@fpistm fpistm self-requested a review April 14, 2023 08:55
@Oliv4945
Copy link
Contributor Author

Ok, updated

STM32 core based on ST HAL automation moved this from In progress to Needs review Apr 14, 2023
@fpistm
Copy link
Member

fpistm commented Apr 14, 2023

Here the patch, if it helps.
Note that you can adapt analog pins if you want as I've defined all possible ones but rename the one as defined by RAK GH repo
patch1.patch

@Oliv4945
Copy link
Contributor Author

Wow, thanks @fpistm and sorry for the mistakes. I have been mislead by another document with some other errors that I reported to RAK. I agree with all your changes except for PB5: unless I am using the wrong datasheet there is no ADC
image
Is that correct?

I order to avoid any further error also doubled checked the PIN definition with their RUI BSP

@fpistm
Copy link
Member

fpistm commented Apr 14, 2023

Oups, you are right. PB5 has no ADC as defined in the generic:

#define PA10 PIN_A0
#define PA11 PIN_A1
#define PA12 PIN_A2
#define PA13 PIN_A3
#define PA14 PIN_A4
#define PA15 PIN_A5
#define PB0 16
#define PB2 PIN_A6
#define PB3 PIN_A7
#define PB4 PIN_A8
#define PB5 20

Please fix.
I've updated my review suggestions. Hope I do no another mistake 😕
And my patch

@fpistm
Copy link
Member

fpistm commented Apr 14, 2023

Could you rebase on top of the main as it seems there are some conflits.

@Oliv4945
Copy link
Contributor Author

Thanks, I applied your patch

@Oliv4945
Copy link
Contributor Author

Could you rebase on top of the main as it seems there are some conflits.

Ok, I just did but I did not saw any conflicts

@fpistm
Copy link
Member

fpistm commented Apr 14, 2023

You don't rebase you do a merge.
Conflict is always here:

This branch cannot be rebased due to conflicts

@Oliv4945
Copy link
Contributor Author

Right I am more used to merge sorry :)
I just rebased and squashed the branch to have cleaner history.

Copy link
Member

@fpistm fpistm left a comment

Choose a reason for hiding this comment

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

Thanks @Oliv4945
LGTM. Nice contribution.
I've not tested as I don't have the RAK module but variant is correctly defined.

STM32 core based on ST HAL automation moved this from Needs review to Reviewer approved Apr 17, 2023
@fpistm fpistm merged commit f76926c into stm32duino:main Apr 17, 2023
22 checks passed
STM32 core based on ST HAL automation moved this from Reviewer approved to Done Apr 17, 2023
@Oliv4945 Oliv4945 deleted the variant/RAK3172-Module branch April 17, 2023 09:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Development

Successfully merging this pull request may close these issues.

None yet

2 participants