Hi, On 14 November 2017 at 03:16, will sanfilippo <[email protected]> wrote:
> +1 Sounds good to me. > > > On Nov 13, 2017, at 5:24 PM, Christopher Collins <[email protected]> > wrote: > > > > 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. > > > That looks good. What I would add here is some kind of compiler warning or similar saying that this is going to change. I know this is not easy however we have some idea here but requires some changes in newt: e.g. 1. We could add special MYNEWT_VAL e.g HAL_GPIO_LEGACY_IRQ_ENABLE) for 1.3 release which would guard clearing interrupt if set to 1 and not clear if 0. 2. This value would have to be set by user so we make sure user is aware knows. Default would be -1 and #error would be propagated if it is -1. 3. In future release we would remove this MYNEWT_VAL and start use new behavior. In this place we need newt change so it notifies that there is MYNEWT_VAL used by user which is not defined. Hope that makes sens :) > > 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 > > Łukasz
