On Mon, Nov 13, 2017 at 04:32:58PM -0800, will sanfilippo wrote:
> Chris:
> 
> Personally, I think there should be separate API as it is more flexible and 
> the API names more accurately describe what the API is doing.
> 
> I do realize that this is more work and given that there currently is no API 
> to clear a pending interrupt, I suspect that everyone who used the enable API 
> expected the interrupt to be cleared prior to enabling.
> 
> Another possible solution (and yes, I suspect folks might think this crazy):
> 
> * Rename the API to something like: hal_gpio_irq_clear_and_enable()
> * Make all implementations consistent and use this API.
> 
> This way we could add the separate API over time and the code will work as 
> expected. Yeah, I know, crazy thought :-)

I don't think that is crazy, but I think it might be a bit disruptive
for some users.  Any code that calls `hal_gpio_irq_enable()` will fail
to build after the rename.  I assume that is the point: make sure things
break loudly if they are going to break.  At the risk of sounding lazy,
that seems like it could be a lot of work for everyone :).  Especially
if we plan on ultimately deprecating `hal_gpio_irq_clear_and_enable()`.

Here is an alternative plan for introducing the API change:

    v1.3:
        - Don't change any implementations of `hal_gpio_irq_enable()`
        - Add the `hal_gpio_set/clear` to the API
        - Notify users that the behavior of `hal_gpio_irq_enable()` will
          be changing in the future (for some MCUs).
        - Modify code in apache-mynewt-core such that it doesn't assume
          that `hal_gpio_irq_enable()` clears the pending interrupt.
          That is, explicitly call the new `clear` function prior to
          enabling the interrupt.

    v1.x [not sure if this is v1.4 or a later release]:
        - Modify implementations of `hal_gpio_irq_enable()` such that
          they only enable the interrupt (don't clear it).

Chris

Reply via email to