Skip to content

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

Merged
merged 1 commit into from
May 4, 2020

Conversation

artokin
Copy link
Contributor

@artokin artokin commented Apr 28, 2020

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

[x] Patch update (Bug fix / Target update / Docs update / Test update / Refactor)
[] Feature update (New feature / Functionality change / New API)
[] Major update (Breaking change E.g. Return code change / API behaviour change)

Test results

[] No Tests required for this change (E.g docs only update)
[x] Covered by existing mbed-os tests (Greentea or Unittest)
[] Tests / results supplied as part of this PR

Reviewers

@JarkkoPaso


@artokin artokin requested a review from JarkkoPaso April 28, 2020 09:35
@@ -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)
Copy link

@MarceloSalazar MarceloSalazar Apr 28, 2020

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

Copy link
Contributor Author

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.

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

Copy link
Contributor

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 ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@mergify mergify bot added the needs: work label Apr 28, 2020
@mbed-ci
Copy link

mbed-ci commented May 4, 2020

Test run: SUCCESS

Summary: 6 of 6 test jobs passed
Build number : 1
Build artifacts

@0xc0170 0xc0170 requested a review from MarceloSalazar May 4, 2020 07:57
Copy link

@MarceloSalazar MarceloSalazar left a 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.

@0xc0170 0xc0170 merged commit ddf06fc into ARMmbed:master May 4, 2020
@mergify mergify bot removed the ready for merge label May 4, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants