Hi, The fix should only skip creating a new timer, but it still should initialize other fields in the callout struct since some code may need to initialize the same callout with e.g. different callback for an event (no idea if we use it like this anywhere). As for the issue itself, the code can be modified so callout is initialized only once (on init) since it's always initialized with the same values - this happens in other places in code as well iirc so it can be improved. Patches are welcome :-)
Best, Andrzej On Sat, Jul 11, 2020 at 3:59 PM <j...@codingfield.com> wrote: > I'm sorry to bump this message, but I would be very interested in your > opinion on this issue, and on the fix I applied in > npl_freertos_callout_init(). It seems to work fine, but I don't know if > it can produce bad side effects... > > Thanks! > > Le 04/07/2020 20:32, j...@codingfield.com a écrit : > > Hi, > > > > I'm still working on my firmware for Pinetime! Thanks to your help a > > couple of weeks ago, BLE connectivity is now much more reliable. > > As a remainder, I work on a firmware based on FreeRTOS and running on > > the NRF52 MCU. It uses NimBLE as BLE stack. I use the FreeRTOS port > > available in the source tree > > (https://github.com/apache/mynewt-nimble/tree/master/porting/npl). > > > > However, I noticed that my code would crash after a certain number of > > connections/disconnections. I noticed that use of the heap memory > > allocated to FreeRTOS would grow by ~100 bytes each time a new > > connection is established and this memory is never released. > > > > I discovered that this memory is used by 2 timers (mynewt "callouts") > > that are created in npl_freertos_callout_init(). This function calls > > xTimerCreate() which allocates the memory necessary for the timer. > > This memory is taken from the heap allocated for FreeRTOS. > > When the connection is closed, xTimerDelete() is never called meaning > > that the memory is not freed. > > When a new device connects, 2 new timers are created, allocating ~100B > > from the heap. This continues until the heap is full and refuses to > > give any more memory. > > Note that I use heap_4.c, which allows memory to be allocated and > > freed. > > > > To my surprise, there is NO function npl_freertos_callout_uninit() or > > npl_freertos_callout_delete() or anything else. It looks like timers > > (callouts) are not intended to be removed once they are created. > > Should they be reused/recycled? > > > > Anyway, I modified the function npl_freertos_callout_init() so that it > > does nothing if the handle is not NULL and... it looks like it works > > (even if I don't think this is a valid fix): > > > > void npl_freertos_callout_init(struct ble_npl_callout *co, struct > > ble_npl_eventq *evq, > > ble_npl_event_fn *ev_cb, void *ev_arg) > > { > > if(co->handle != NULL) return; // <---- I added this line. > > > > memset(co, 0, sizeof(*co)); > > co->handle = xTimerCreate("co", 1, pdFALSE, co, > > os_callout_timer_cb); > > co->evq = evq; > > ble_npl_event_init(&co->ev, ev_cb, ev_arg); > > > > } > > > > Is this a bug that must be fixed in the source code or am I missing > > something about the creation and deletion of timers? > > > > Thanks for your help, > > > > JF >
