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

✨ stm32f1 ADC driver #77

Merged
merged 14 commits into from
Jan 29, 2025
Merged

✨ stm32f1 ADC driver #77

merged 14 commits into from
Jan 29, 2025

Conversation

50xp50
Copy link
Contributor

@50xp50 50xp50 commented Dec 17, 2024

No description provided.

@50xp50 50xp50 force-pushed the STM32F1_ADC_Driver branch 2 times, most recently from 7551d56 to fbd7cf5 Compare December 17, 2024 19:50
@kammce
Copy link
Member

kammce commented Dec 18, 2024

Break the adc demo changes out to a new branch. I haven't come up with a solid way to manage this otherwise. But basically demos, must follow what is available in package releases. Meaning that we cannot commit changes to demos until a package has been released.

@50xp50 50xp50 force-pushed the STM32F1_ADC_Driver branch from fbd7cf5 to 52be489 Compare December 18, 2024 19:26
@50xp50
Copy link
Contributor Author

50xp50 commented Dec 18, 2024

Fixed. Will do separate PR with demo portion once merged.

kammce
kammce requested changes Dec 19, 2024
View reviewed changes
Copy link
Member

@kammce kammce left a comment

Choose a reason for hiding this comment

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

Great work, but will need some changes. I'm also not sure how this works if you have multiple ADC pins in use at once.

include/libhal-stm32f1/adc.hpp Outdated Show resolved Hide resolved
src/stm32f1/adc.cpp Outdated Show resolved Hide resolved
src/stm32f1/adc_reg.hpp Outdated Show resolved Hide resolved
src/stm32f1/adc.cpp Outdated Show resolved Hide resolved
include/libhal-arm-mcu/stm32f1/adc.hpp Outdated Show resolved Hide resolved
include/libhal-arm-mcu/stm32f1/adc.hpp Outdated Show resolved Hide resolved
src/stm32f1/adc.cpp Outdated Show resolved Hide resolved
src/stm32f1/adc.cpp Outdated Show resolved Hide resolved
src/stm32f1/adc.cpp Outdated Show resolved Hide resolved
src/stm32f1/adc_reg.hpp Outdated Show resolved Hide resolved
kammce
kammce requested changes Dec 23, 2024
View reviewed changes
Copy link
Member

@kammce kammce left a comment

Choose a reason for hiding this comment

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

Looking fantastic. Just a few more things and we'll be good to go.

include/libhal-arm-mcu/stm32f1/adc.hpp Outdated Show resolved Hide resolved
include/libhal-arm-mcu/stm32f1/adc.hpp Outdated Show resolved Hide resolved
src/stm32f1/adc.cpp Outdated Show resolved Hide resolved
src/stm32f1/adc.cpp Outdated Show resolved Hide resolved
src/stm32f1/adc.cpp Outdated Show resolved Hide resolved
@kammce
Copy link
Member

kammce commented Dec 23, 2024

I might have you implement the locking mechanism for this and acquire_. I see it as a low cost addition that also makes these apis safer to use without minding anything.

@50xp50
Copy link
Contributor Author

50xp50 commented Jan 2, 2025

I might have you implement the locking mechanism for this and acquire_. I see it as a low cost addition that also makes these apis safer to use without minding anything.

This is only available once 5.0 releases correct?

@50xp50 50xp50 requested a review from kammce January 2, 2025 19:42
@kammce
Copy link
Member

kammce commented Jan 7, 2025

I might have you implement the locking mechanism for this and acquire_. I see it as a low cost addition that also makes these apis safer to use without minding anything.

This is only available once 5.0 releases correct?

No this can be done on any version. Its a stylistic approach.

50xp50 and others added 6 commits January 21, 2025 16:12
@50xp50
✨ stm32f1 adc peripheral manager
552c989
@50xp50
Merge branch 'libhal:main' into STM32F1_ADC_Driver
583bd5a
@50xp50
✨ stm32f1 ADC driver
249dc3f
@50xp50
✨ stm32f1 adc peripheral manager
8df56a9 
- Updated to fix register pointer error.
- Verified and tested with modified demo using both adc1 and adc2,
as well as using adc1 with 2 separate channels.
@50xp50
✨ stm32f1 adc peripheral manager
46a26dc 
- Updated to fix register pointer error.
- Verified and tested with modified demo using both adc1 and adc2,
as well as using adc1 with 2 separate channels.
@50xp50
✨ stm32f1 adc peripheral manager
3234dd2 
- Updated to fix register pointer error.
- Verified and tested with modified demo using both adc1 and adc2,
as well as using adc1 with 2 separate channels.
@50xp50
Copy link
Contributor Author

50xp50 commented Jan 25, 2025

Resolves #7. Updated to add the adc peripheral manager and acquire_ styling. Also tested and verified using both adc's in the demo together, as well as using multiple channels on the same one. Will do separate PR with updated demo once pulled

kammce
kammce requested changes Jan 26, 2025
View reviewed changes
conanfile.py Outdated Show resolved Hide resolved
include/libhal-arm-mcu/stm32f1/adc.hpp Outdated Show resolved Hide resolved
include/libhal-arm-mcu/stm32f1/adc.hpp Outdated Show resolved Hide resolved
include/libhal-arm-mcu/stm32f1/adc.hpp Outdated Show resolved Hide resolved
include/libhal-arm-mcu/stm32f1/adc.hpp Outdated Show resolved Hide resolved
src/stm32f1/adc.cpp Show resolved Hide resolved
src/stm32f1/adc.cpp Outdated Show resolved Hide resolved
src/stm32f1/adc.cpp Outdated Show resolved Hide resolved
src/stm32f1/adc.cpp Show resolved Hide resolved
src/stm32f1/adc.cpp Outdated Show resolved Hide resolved
50xp50 added 2 commits January 27, 2025 12:44
@50xp50
🎨 Updated to reflect PR changes
0b0bd79 
- Changed from adc_reg_t * to void * for adc_reg_location
	- Included function to cast for usage in implementation file
- Updated to make classes non-copyable or movable
- Removal/addition of some const's
- Fixed comments
@50xp50
🎨 Fixed formatting
3922377
kammce
kammce approved these changes Jan 27, 2025
View reviewed changes
src/stm32f1/adc.cpp Outdated Show resolved Hide resolved
kammce
kammce requested changes Jan 28, 2025
View reviewed changes
Copy link
Member

@kammce kammce left a comment

Choose a reason for hiding this comment

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

Just a few last things to work on and we should be golden. I'll bypass the docs issue and submit even if it fails.

include/libhal-arm-mcu/stm32f1/adc.hpp Outdated Show resolved Hide resolved
include/libhal-arm-mcu/stm32f1/adc.hpp Outdated Show resolved Hide resolved
include/libhal-arm-mcu/stm32f1/adc.hpp Outdated Show resolved Hide resolved
include/libhal-arm-mcu/stm32f1/adc.hpp Outdated Show resolved Hide resolved
include/libhal-stm32f1/adc.hpp Outdated Show resolved Hide resolved
src/stm32f1/adc.cpp Outdated Show resolved Hide resolved
kammce
kammce approved these changes Jan 29, 2025
View reviewed changes
Copy link
Member

@kammce kammce left a comment

Choose a reason for hiding this comment

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

Great work @50xp50 ! Approved!

@kammce kammce merged commit c78468a into libhal:main Jan 29, 2025
16 of 17 checks passed
@kammce kammce mentioned this pull request Feb 5, 2025
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kammce kammce kammce approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@50xp50 @kammce