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.

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

Change mutex to PlatformMutex #70

Merged
merged 1 commit into from
Nov 20, 2017
Merged

Change mutex to PlatformMutex #70

merged 1 commit into from
Nov 20, 2017

Conversation

deepikabhavnani
Copy link

Resolves: #69

@deepikabhavnani
Copy link
Author

@geky - Please review

Copy link
Contributor

@geky geky left a comment

Choose a reason for hiding this comment

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

I'm not sure we really needed the lock functions, but it's fine with me

@deepikabhavnani
Copy link
Author

deepikabhavnani commented Nov 20, 2017

@geky - Lock/Unlock functions are not required, added just to have consistent code with mbed-os drivers.

@deepikabhavnani deepikabhavnani merged commit 5df3329 into PelionIoT:master Nov 20, 2017
@deepikabhavnani deepikabhavnani deleted the mutex_rtos branch November 20, 2017 22:50
@geky
Copy link
Contributor

geky commented Nov 20, 2017

Hmm, I think most drivers have lock + unlock so that multiple consumers of the class can synchronize on hardware accesses. (Like SPI transactions).

@deepikabhavnani
Copy link
Author

In that case, lock/unlock functions should be public right?

@geky
Copy link
Contributor

geky commented Nov 20, 2017

I think the idea is you can extend the classes to provide a different synchronization interface. But for a non-transactional class each function is its own synchronization boundary.

cc @c1728p9 for his thoughts

@deepikabhavnani deepikabhavnani restored the mutex_rtos branch November 22, 2017 19:56
@deepikabhavnani deepikabhavnani deleted the mutex_rtos branch November 22, 2017 20:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants