FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

Add RAK3172 module by Oliv4945 · Pull Request #2005 · stm32duino/Arduino_Core_STM32 · GitHub

/ Arduino_Core_STM32 Public

Add RAK3172 module - #2005

Merged
fpistm merged 1 commit into
stm32duino:mainfrom
Oliv4945:variant/RAK3172-Module
Apr 17, 2023
ConversationCommits (1)ChecksFiles changed
Merged

Add RAK3172 module#2005
fpistm merged 1 commit into
stm32duino:mainfrom
Oliv4945:variant/RAK3172-Module

Conversation

Oliv4945 commented Apr 13, 2023

Copy link
Copy Markdown
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 commented Apr 13, 2023

Copy link
Copy Markdown
Member

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

Oliv4945 commented Apr 14, 2023

Copy link
Copy Markdown
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 added the new variant Add support of new bard label Apr 14, 2023
fpistm added this to the 2.6.0 milestone Apr 14, 2023

fpistm commented Apr 14, 2023

Copy link
Copy Markdown
Member

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

fpistm self-requested a review April 14, 2023 08:55

Oliv4945 commented Apr 14, 2023

Copy link
Copy Markdown
Contributor Author

Ok, updated

fpistm requested changes Apr 14, 2023
View reviewed changes
Comment thread variants/STM32WLxx/WL54CCU_WL55CCU_WLE4C(8-B-C)U_WLE5C(8-B-C)U/variant_RAK3172_MODULE.h Outdated
Comment thread variants/STM32WLxx/WL54CCU_WL55CCU_WLE4C(8-B-C)U_WLE5C(8-B-C)U/variant_RAK3172_MODULE.h Outdated
Comment thread variants/STM32WLxx/WL54CCU_WL55CCU_WLE4C(8-B-C)U_WLE5C(8-B-C)U/variant_RAK3172_MODULE.h Outdated
Comment thread variants/STM32WLxx/WL54CCU_WL55CCU_WLE4C(8-B-C)U_WLE5C(8-B-C)U/variant_RAK3172_MODULE.cpp Outdated
Comment thread variants/STM32WLxx/WL54CCU_WL55CCU_WLE4C(8-B-C)U_WLE5C(8-B-C)U/variant_RAK3172_MODULE.cpp Outdated

fpistm commented Apr 14, 2023 •
edited
Loading

Copy link
Copy Markdown
Member

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 commented Apr 14, 2023

Copy link
Copy Markdown
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

Is that correct?

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

fpistm commented Apr 14, 2023 •
edited
Loading

Copy link
Copy Markdown
Member

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 commented Apr 14, 2023

Copy link
Copy Markdown
Member

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

Oliv4945 commented Apr 14, 2023

Copy link
Copy Markdown
Contributor Author

Thanks, I applied your patch

Oliv4945 commented Apr 14, 2023

Copy link
Copy Markdown
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 commented Apr 14, 2023

Copy link
Copy Markdown
Member

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

This branch cannot be rebased due to conflicts

Oliv4945 force-pushed the variant/RAK3172-Module branch from 029f9d6 to 3874dac Compare April 16, 2023 06:24

Oliv4945 commented Apr 16, 2023

Copy link
Copy Markdown
Contributor Author

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

fpistm approved these changes Apr 17, 2023
View reviewed changes

fpistm left a comment •
edited
Loading

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

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

fpistm merged commit f76926c into stm32duino:main Apr 17, 2023
Oliv4945 deleted the variant/RAK3172-Module branch April 17, 2023 09:16
fpistm mentioned this pull request Oct 16, 2023
Closed
fpistm mentioned this pull request Jul 15, 2024
Open
93 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

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

Back | FazBrowse Home | New Git URL