-
Notifications
You must be signed in to change notification settings - Fork 3k
802.15.4 STM S2LP driver update #12876
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
Conversation
Sync with v1.0.3
@@ -28,7 +28,7 @@ | |||
// Uncomment to use testing gpios attached to TX/RX processes | |||
// #define TEST_GPIOS_ENABLED | |||
|
|||
#if defined(TARGET_MTB_STM_S2LP) | |||
#if defined(TARGET_MTB_STM_S2LP) || defined(TARGET_MTB_STM_S2LP_CT) | |||
#if !defined(S2LP_SPI_SDI) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe we shouldn't add custom configuration/defines to the drivers.
Instead, please use mbed_app.json to configure pin config in the application - this will let us remove target dependencies in Mbed OS.
See example for esp8266-driver
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@MarceloSalazar , can configuration/define change be done in another PR?
We would like to enable custom target first to verify we get target to our CI correctly. When that phase is completed we could continue with additional optimisation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You certainly can, but we need to move away from custom config in drivers asap and this file will be modified again. Every PR requires review, testing and time ;-)
@0xc0170 let us know if you think we can proceed as it's
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As it is already there , lets get this in and will be fixed - @artokin can you create a tracking issue for this ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure, the issue is: https://jira.arm.com/browse/IOTTHD-4097
Test run: SUCCESSSummary: 6 of 6 test jobs passed |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's go ahead as it's. Target removal will be done on separate PR.
Summary of changes
Sync with v1.0.3 in master repository.
Allow usage of custom target TARGET_MTB_STM_S2LP_CT.
Impact of changes
Migration actions required
Documentation
Pull request type
Test results
Reviewers
@JarkkoPaso