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

initialization approach for STM32.Button #23

Closed
pat-rogers opened this issue May 20, 2016 · 3 comments
Closed

initialization approach for STM32.Button #23

pat-rogers opened this issue May 20, 2016 · 3 comments

Comments

@pat-rogers
Copy link
Contributor

Currently there is a check within the Has_Been_Pressed function to see if the hardware has been configured via procedure Initialize. If not, the function calls Initialize. That has the advantage of not initializing unless needed, but doesn't seem worth the overhead on every function call, some of which might be time-critical.

If the user has the package in the closure of context clauses, surely they intend to use the package, and so initialization is required. Currently all the code in the repo does the initialize call explicitly.

We could have the package body executable part call Initialize. That would guarantee initialization, and would only be done once. We wouldn't need to export procedure Initialize in that case. However then we have to worry about access-before-elaboration issues, and for that reason I generally recommend people not do calls to other library packages during lib unit elaboration. An explicit initialization routine gives clients full control over the order of initialization.

In addition, having an exported initialization routine would allow the user to specify (with a new parameter) the interrupt rising or falling edge choice. Currently we hardcode it to rising edge.

If we require clients to call Initialize (ie, remove the check and the call from within the function), they might forget to do it. We could address that by having a precondition on the function that would make it clear that it is required, and check that it has in fact been initialized. The precondition check would only be enabled in debug mode when it wouldn't hurt performance, and then users who forget to call Initialize will see what they missed. Just seeing the precondition in the package spec is likely sufficient to indicate what they forgot, even without debugging.

IMO the explicit initialization procedure is the best approach, with the precondition on the function.

@lambourg
Copy link
Member

I agree we should just add a precondition here, that would allow testing that everything is fine in debug mode, and remove the overhead in production mode. That's clearly what preconditions are for, so I'm all for it.

I wouldn't want to have the initialization performed during the elaboration because this would be inconsistent with other packages whose initialize procedures have user-defined parameters and so cannot be performed automatically.

@Fabien-Chouteau
Copy link
Member

I agree as well. We should standardize the board features packages (LEDs, buttons, screen, gyro, features of expansion boards, etc.) to have a set of common sub-programs:

type Init_Status is (Init_Ok, 
                     --  When everything is ok

                     Init_Not_Available,
                     --  When the feature is not available, for instance if the extension board is not plugged-in

                     Init_Error);
                     --  When everything went wrong...
procedure Initialize return Init_Status;
function Initialized return Boolean;

pat-rogers added a commit that referenced this issue May 21, 2016
…with pre/postconditions.

Add parameter to Initialize so that user can select button interrupt rising or falling edge,
with default to True so that current callers can keep behavior without source changes.

See discussion Issue #23.
@pat-rogers
Copy link
Contributor Author

Done.

A common initialization would be useful.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants