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 :-) > On Nov 13, 2017, at 3:25 PM, Christopher Collins <[email protected]> wrote: > > Hello all, > > It appears there is some inconsistency among implementations of the > `hal_gpio_irq_enable()` function. There are two behaviors associated > with this function: > > (1) Enable the interrupt associated with the specified pin. > (2) Clear any pending interrupt associated with the specified pin. > > All implementations perform (1); only some perform (2). > > This email concerns the second item - should `hal_gpio_irq_enable()` > also clear the pending interrupt? > > I can understand why it is useful for the function to perform both > actions. Often, when some code temporarily disables an interrupt, it > wants to ensure that an interrupt triggered in the meantime does not > cause its ISR to be executed. Since there is no HAL function to clear > interrupt state, the enable function is the natural place to implement > this behavior. > > On the other hand, it could argued that a function which does both > things is too coarse an abstraction. An application may want to perform > just one of the two actions. > > In my view, there are two possible solutions: > > 1. Just make the behavior consistent: make all implementations of > `hal_gpio_irq_enable()` perform both actions. > > 2. Add some functions to the HAL: > a. Make all implementations of `hal_gpio_irq_enable()` only perform > the first action. > b. Add `void hal_gpio_irq_clear(int pin)` > c. Add `void hal_gpio_irq_set(int pin)` > > (or combine b and c into a single function.) > > Option 2 introduces more of a backwards compatibility headache than 1 > does. Still, I think it is the better solution. > > All comments welcome. > > Thanks, > Chris
