scb: add static version of system_reset as sys_reset

[hdhoang]

As suggested in https://github.com/ah-/anne-key/pull/94. I branched this off v0.5.8 to verify the function in that PR, and against rtfm v0.3 in hdhoang/anne-key@d6fb831

I have cloned the body of system_reset, do you think we should call one from the other (e.g. ignoring self in system_reset, or stealing Peripherals in system_reset2)?

[rust-highfive]

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @ithinuel (or someone else) soon.

If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes.

Please see the contribution instructions for more information.

[hdhoang]

hi team! could someone take a look?

[korken89]

Hi, overall I think this looks good, however I'm not 100% sure of the name.
But I have no strong opinion there, the documentation says that it is a static version.

Any comments against @rust-embedded/cortex-m ? Else I will approve and merge this today.

[adamgreig]

Much as I hate to suggest a new breaking change so soon after 0.6, I wonder if system_reset would be better as a single static function, so remove self from the current system_reset and don't have system_reset2. Perhaps we could do that in 0.7, mark system_reset as deprecated, and maybe call the new static function sys_reset rather than system_reset2?

[azasypkin]

Perhaps we could do that in 0.7, mark system_reset as deprecated, and maybe call the new static function sys_reset rather than system_reset2?

Sorry for hijacking, but if we go this route, it'd probably make sense to do the same for set_deepsleep/clear_deepsleep as briefly discussed in https://github.com/japaric/cortex-m-rtfm/issues/165. It will be a bit more difficult to come up with new names in this case though.

[korken89]

I agree on only having a static version as it makes sense.
But it does not hurt to have both and backwards compatibility, so I have no strong opinion on deprecating the existing API.

What does @therealprof or @japaric think?

[japaric]

IMO, it makes sense to add this static method and deprecate the old system_reset method using the #[deprecated] attribute in a new patch version (i.e. v0.6.1). (Deprecating things does not warrant a semver bump -- yes, deprecating things will trigger deny(warning) errors but only of things you have the source code at hand; crates that come from crates.io will not error (module compiler bugs))

[hdhoang]

I could add the deprecation warning and a TODO to rename the system_reset2 as well. The same method would also apply cleanly to static deepsleep functions.

[adamgreig]

Are you suggesting we call the new static function system_reset2 for now, then once we've removed system_reset and replaced it with the new static function, we leave a proxy system_reset2 in but deprecated and remove it in another release?

It's sort of nice in that we end up with just system_reset as a static function but it does mean two deprecated items/breaking changes. It would also help avoid needing to find new names for the deepsleep functions.

[therealprof]

I'm not too attached to system_reset so adding sys_reset and deprecating the former is okay for me.

[hdhoang]

I have incorporated your feedbacks, thanks!

[korken89]

I'm also quite ok with what @therealprof said

[korken89]

I think this is close enough for naming, we can bike-shed this forever :)

[korken89]

bors r+

[bors]

Build succeeded

• continuous-integration/travis-ci/push