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.

Sign up for GitHub

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

Jump to bottom

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 the new variant Add support of new bard label 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

@fpistm
Copy link
Member

fpistm commented Apr 14, 2023 •
edited
Loading

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 •
edited
Loading

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

Arduino_Core_STM32/variants/STM32WLxx/WL54CCU_WL55CCU_WLE4C(8-B-C)U_WLE5C(8-B-C)U/variant_generic.h

Lines 28 to 38 in 31e562d

#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 Oliv4945 force-pushed the variant/RAK3172-Module branch from 029f9d6 to 3874dac Compare April 16, 2023 06:24
@Oliv4945
Copy link
Contributor Author

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

fpistm
fpistm approved these changes Apr 17, 2023
View reviewed changes
Copy link
Member

@fpistm fpistm left a comment •
edited
Loading

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.

@fpistm fpistm merged commit f76926c into stm32duino:main Apr 17, 2023
@Oliv4945 Oliv4945 deleted the variant/RAK3172-Module branch April 17, 2023 09:16
@fpistm fpistm mentioned this pull request Oct 16, 2023
Closed
@fpistm fpistm mentioned this pull request Jul 15, 2024
Open
83 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@fpistm fpistm fpistm approved these changes

Assignees
No one assigned
Labels
new variant Add support of new bard
Projects
None yet
Milestone
2.6.0
Development

Successfully merging this pull request may close these issues.
2 participants
@Oliv4945 @fpistm