From e2b019edd9b1c9655d0fce0d3e077a24e736d77e Mon Sep 17 00:00:00 2001 From: Stacey Sheldon Date: Sat, 10 Nov 2012 18:04:16 -0500 Subject: [PATCH 1/4] telemetry: stop registering periodic events for non-periodic UAVOs Telemetry module was iterating over all UAVOs including meta UAVOs and creating a periodic event item for each object. These items cost us about 32 bytes for each list item. This is wasteful for two main reasons. First, meta UAVOs can't meaningfully have periodic updates so excluding them entirely makes sense. That halves the number of objects in this list since there is one meta object for every data object. This is worth about 500 bytes of RAM on CC. Second, about half of the remaining UAVOs are not periodic by default so they're wasting memory unless someone happens to want to make them periodic at runtime. This is worth another 450 bytes of RAM on CC. So, objects that are configured as periodic during board init will support all of the periodic config at runtime. Objects that are *not* periodic during board init can only be made periodic on the next boot. Each object that you make periodic during init will cost you an extra 32 bytes of RAM. With erased settings, free RAM comparison for CC is: Before: 2736 bytes free After: 4048 bytes free Total RAM savings with this update is 1312 bytes! --- flight/Modules/Telemetry/telemetry.c | 89 ++++++++++++++-------------- 1 file changed, 46 insertions(+), 43 deletions(-) diff --git a/flight/Modules/Telemetry/telemetry.c b/flight/Modules/Telemetry/telemetry.c index 240c9b842..44e53236e 100644 --- a/flight/Modules/Telemetry/telemetry.c +++ b/flight/Modules/Telemetry/telemetry.c @@ -74,7 +74,6 @@ static void telemetryRxTask(void *parameters); static int32_t transmitData(uint8_t * data, int32_t length); static void registerObject(UAVObjHandle obj); static void updateObject(UAVObjHandle obj, int32_t eventType); -static int32_t addObject(UAVObjHandle obj); static int32_t setUpdatePeriod(UAVObjHandle obj, int32_t updatePeriodMs); static void processObjEvent(UAVObjEvent * ev); static void updateTelemetryStats(); @@ -154,11 +153,31 @@ MODULE_INITCALL(TelemetryInitialize, TelemetryStart) */ static void registerObject(UAVObjHandle obj) { - // Setup object for periodic updates - addObject(obj); + if (UAVObjIsMetaobject(obj)) { + /* Only connect change notifications for meta objects. No periodic updates */ + UAVObjConnectQueue(obj, priorityQueue, EV_MASK_ALL_UPDATES); + return; + } else { + UAVObjMetadata metadata; + UAVObjUpdateMode updateMode; + UAVObjGetMetadata(obj, &metadata); + updateMode = UAVObjGetTelemetryUpdateMode(&metadata); - // Setup object for telemetry updates - updateObject(obj, EV_NONE); + /* Only create a periodic event for objects that are periodic */ + if ((updateMode == UPDATEMODE_PERIODIC) || + (updateMode == UPDATEMODE_THROTTLED)) { + // Setup object for periodic updates + UAVObjEvent ev = { + .obj = obj, + .instId = UAVOBJ_ALL_INSTANCES, + .event = EV_UPDATED_PERIODIC, + }; + EventPeriodicQueueCreate(&ev, queue, 0); + } + + // Setup object for telemetry updates + updateObject(obj, EV_NONE); + } } /** @@ -171,30 +190,35 @@ static void updateObject(UAVObjHandle obj, int32_t eventType) UAVObjUpdateMode updateMode; int32_t eventMask; + if (UAVObjIsMetaobject(obj)) { + /* This function updates the periodic updates for the object. + * Meta Objects cannot have periodic updates. + */ + PIOS_Assert(false); + return; + } + // Get metadata UAVObjGetMetadata(obj, &metadata); updateMode = UAVObjGetTelemetryUpdateMode(&metadata); // Setup object depending on update mode - if (updateMode == UPDATEMODE_PERIODIC) { + switch (updateMode) { + case UPDATEMODE_PERIODIC: // Set update period setUpdatePeriod(obj, metadata.telemetryUpdatePeriod); // Connect queue eventMask = EV_UPDATED_PERIODIC | EV_UPDATED_MANUAL | EV_UPDATE_REQ; - if (UAVObjIsMetaobject(obj)) { - eventMask |= EV_UNPACKED; // we also need to act on remote updates (unpack events) - } UAVObjConnectQueue(obj, priorityQueue, eventMask); - } else if (updateMode == UPDATEMODE_ONCHANGE) { + break; + case UPDATEMODE_ONCHANGE: // Set update period setUpdatePeriod(obj, 0); // Connect queue eventMask = EV_UPDATED | EV_UPDATED_MANUAL | EV_UPDATE_REQ; - if (UAVObjIsMetaobject(obj)) { - eventMask |= EV_UNPACKED; // we also need to act on remote updates (unpack events) - } UAVObjConnectQueue(obj, priorityQueue, eventMask); - } else if (updateMode == UPDATEMODE_THROTTLED) { + break; + case UPDATEMODE_THROTTLED: if ((eventType == EV_UPDATED_PERIODIC) || (eventType == EV_NONE)) { // If we received a periodic update, we can change back to update on change eventMask = EV_UPDATED | EV_UPDATED_MANUAL | EV_UPDATE_REQ; @@ -205,19 +229,15 @@ static void updateObject(UAVObjHandle obj, int32_t eventType) // Otherwise, we just received an object update, so switch to periodic for the timeout period to prevent more updates eventMask = EV_UPDATED_PERIODIC | EV_UPDATED_MANUAL | EV_UPDATE_REQ; } - if (UAVObjIsMetaobject(obj)) { - eventMask |= EV_UNPACKED; // we also need to act on remote updates (unpack events) - } UAVObjConnectQueue(obj, priorityQueue, eventMask); - } else if (updateMode == UPDATEMODE_MANUAL) { + break; + case UPDATEMODE_MANUAL: // Set update period setUpdatePeriod(obj, 0); // Connect queue eventMask = EV_UPDATED_MANUAL | EV_UPDATE_REQ; - if (UAVObjIsMetaobject(obj)) { - eventMask |= EV_UNPACKED; // we also need to act on remote updates (unpack events) - } UAVObjConnectQueue(obj, priorityQueue, eventMask); + break; } } @@ -271,11 +291,11 @@ static void processObjEvent(UAVObjEvent * ev) // If this is a metaobject then make necessary telemetry updates if (UAVObjIsMetaobject(ev->obj)) { updateObject(UAVObjGetLinkedObj(ev->obj), EV_NONE); // linked object will be the actual object the metadata are for - } - - if((updateMode == UPDATEMODE_THROTTLED) && !UAVObjIsMetaobject(ev->obj)) { - // If this is UPDATEMODE_THROTTLED, the event mask changes on every event. - updateObject(ev->obj, ev->event); + } else { + if (updateMode == UPDATEMODE_THROTTLED) { + // If this is UPDATEMODE_THROTTLED, the event mask changes on every event. + updateObject(ev->obj, ev->event); + } } } } @@ -380,23 +400,6 @@ static int32_t transmitData(uint8_t * data, int32_t length) } } -/** - * Setup object for periodic updates. - * \param[in] obj The object to update - * \return 0 Success - * \return -1 Failure - */ -static int32_t addObject(UAVObjHandle obj) -{ - UAVObjEvent ev; - - // Add object for periodic updates - ev.obj = obj; - ev.instId = UAVOBJ_ALL_INSTANCES; - ev.event = EV_UPDATED_PERIODIC; - return EventPeriodicQueueCreate(&ev, queue, 0); -} - /** * Set update period of object (it must be already setup for periodic updates) * \param[in] obj The object to update From e5c54cca0041905b04973114f8ac057f7646c743 Mon Sep 17 00:00:00 2001 From: Stacey Sheldon Date: Sun, 11 Nov 2012 22:16:00 -0500 Subject: [PATCH 2/4] freertos: change default alignment to 4-byte from 8-byte There shouldn't be any reason to need 8-byte alignment on the F1 platform. This allows better packing of all malloc'd data. Reducing this below 4-byte alignment is not recommended and will likely result in misaligned pointers being passed to peripherals. RAM savings is another 300 bytes. --- .../Libraries/FreeRTOS/Source/portable/GCC/ARM_CM3/portmacro.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/flight/PiOS/STM32F10x/Libraries/FreeRTOS/Source/portable/GCC/ARM_CM3/portmacro.h b/flight/PiOS/STM32F10x/Libraries/FreeRTOS/Source/portable/GCC/ARM_CM3/portmacro.h index c2860d1e9..910ff770a 100644 --- a/flight/PiOS/STM32F10x/Libraries/FreeRTOS/Source/portable/GCC/ARM_CM3/portmacro.h +++ b/flight/PiOS/STM32F10x/Libraries/FreeRTOS/Source/portable/GCC/ARM_CM3/portmacro.h @@ -92,7 +92,7 @@ extern "C" { /* Architecture specifics. */ #define portSTACK_GROWTH ( -1 ) #define portTICK_RATE_MS ( ( portTickType ) 1000 / configTICK_RATE_HZ ) -#define portBYTE_ALIGNMENT 8 +#define portBYTE_ALIGNMENT 4 /*-----------------------------------------------------------*/ From c1fc60569683f409b51be49aa178687209406b5d Mon Sep 17 00:00:00 2001 From: Stacey Sheldon Date: Wed, 14 Nov 2012 23:38:24 -0500 Subject: [PATCH 3/4] uavobjectmanager: remove linked list for UAVOs --- flight/UAVObjects/uavobjectmanager.c | 47 +++++++++++---------------- flight/UAVObjects/uavobjecttemplate.c | 2 +- 2 files changed, 20 insertions(+), 29 deletions(-) diff --git a/flight/UAVObjects/uavobjectmanager.c b/flight/UAVObjects/uavobjectmanager.c index c339af0fe..7938f99c3 100644 --- a/flight/UAVObjects/uavobjectmanager.c +++ b/flight/UAVObjects/uavobjectmanager.c @@ -41,6 +41,17 @@ // Macros #define SET_BITS(var, shift, value, mask) var = (var & ~(mask << shift)) | (value << shift); +/* Table of UAVO handles registered at compile time */ +extern struct UAVOData * __start__uavo_handles __attribute__((weak)); +extern struct UAVOData * __stop__uavo_handles __attribute__((weak)); + +#define UAVO_LIST_ITERATE(_item) \ +for (struct UAVOData ** _uavo_slot = &__start__uavo_handles; \ + _uavo_slot <= &__stop__uavo_handles; \ + _uavo_slot++) { \ + struct UAVOData * _item = *_uavo_slot; \ + if (_item == NULL) continue; + /** * List of event queues and the eventmask associated with the queue. */ @@ -98,7 +109,6 @@ struct UAVOData { * inside the payload for this UAVO. */ struct UAVOMeta metaObj; - struct UAVOData * next; uint16_t instance_size; } __attribute__((packed)); @@ -164,7 +174,6 @@ static void customSPrintf(uint8_t * buffer, uint8_t * format, ...); #endif // Private variables -static struct UAVOData * uavo_list; static xSemaphoreHandle mutex; static const UAVObjMetadata defMetadata = { .flags = (ACCESS_READWRITE << UAVOBJ_ACCESS_SHIFT | @@ -188,7 +197,6 @@ static UAVObjStats stats; int32_t UAVObjInitialize() { // Initialize variables - uavo_list = NULL; memset(&stats, 0, sizeof(UAVObjStats)); // Create mutex @@ -338,9 +346,6 @@ UAVObjHandle UAVObjRegister(uint32_t id, /* Initialize the embedded meta UAVO */ UAVObjInitMetaData (&uavo_data->metaObj); - /* Add the newly created object to the global list of objects */ - LL_APPEND(uavo_list, uavo_data); - /* Initialize object fields and metadata to default values */ if (initCb) initCb((UAVObjHandle) uavo_data, 0); @@ -374,8 +379,7 @@ UAVObjHandle UAVObjGetByID(uint32_t id) xSemaphoreTakeRecursive(mutex, portMAX_DELAY); // Look for object - struct UAVOData * tmp_obj; - LL_FOREACH(uavo_list, tmp_obj) { + UAVO_LIST_ITERATE(tmp_obj) if (tmp_obj->id == id) { found_obj = (UAVObjHandle *)tmp_obj; goto unlock_exit; @@ -1019,15 +1023,13 @@ int32_t UAVObjDelete(UAVObjHandle obj_handle, uint16_t instId) */ int32_t UAVObjSaveSettings() { - struct UAVOData *obj; - // Get lock xSemaphoreTakeRecursive(mutex, portMAX_DELAY); int32_t rc = -1; // Save all settings objects - LL_FOREACH(uavo_list, obj) { + UAVO_LIST_ITERATE(obj) // Check if this is a settings object if (UAVObjIsSettings(obj)) { // Save object @@ -1051,15 +1053,13 @@ unlock_exit: */ int32_t UAVObjLoadSettings() { - struct UAVOData *obj; - // Get lock xSemaphoreTakeRecursive(mutex, portMAX_DELAY); int32_t rc = -1; // Load all settings objects - LL_FOREACH(uavo_list, obj) { + UAVO_LIST_ITERATE(obj) // Check if this is a settings object if (UAVObjIsSettings(obj)) { // Load object @@ -1083,15 +1083,13 @@ unlock_exit: */ int32_t UAVObjDeleteSettings() { - struct UAVOData *obj; - // Get lock xSemaphoreTakeRecursive(mutex, portMAX_DELAY); int32_t rc = -1; // Save all settings objects - LL_FOREACH(uavo_list, obj) { + UAVO_LIST_ITERATE(obj) // Check if this is a settings object if (UAVObjIsSettings(obj)) { // Save object @@ -1115,15 +1113,13 @@ unlock_exit: */ int32_t UAVObjSaveMetaobjects() { - struct UAVOData *obj; - // Get lock xSemaphoreTakeRecursive(mutex, portMAX_DELAY); int32_t rc = -1; // Save all settings objects - LL_FOREACH(uavo_list, obj) { + UAVO_LIST_ITERATE(obj) // Save object if (UAVObjSave( (UAVObjHandle) MetaObjectPtr(obj), 0) == -1) { @@ -1144,15 +1140,13 @@ unlock_exit: */ int32_t UAVObjLoadMetaobjects() { - struct UAVOData *obj; - // Get lock xSemaphoreTakeRecursive(mutex, portMAX_DELAY); int32_t rc = -1; // Load all settings objects - LL_FOREACH(uavo_list, obj) { + UAVO_LIST_ITERATE(obj) // Load object if (UAVObjLoad((UAVObjHandle) MetaObjectPtr(obj), 0) == -1) { @@ -1173,15 +1167,13 @@ unlock_exit: */ int32_t UAVObjDeleteMetaobjects() { - struct UAVOData *obj; - // Get lock xSemaphoreTakeRecursive(mutex, portMAX_DELAY); int32_t rc = -1; // Load all settings objects - LL_FOREACH(uavo_list, obj) { + UAVO_LIST_ITERATE(obj) // Load object if (UAVObjDelete((UAVObjHandle) MetaObjectPtr(obj), 0) == -1) { @@ -1787,8 +1779,7 @@ void UAVObjIterate(void (*iterator) (UAVObjHandle obj)) xSemaphoreTakeRecursive(mutex, portMAX_DELAY); // Iterate through the list and invoke iterator for each object - struct UAVOData *obj; - LL_FOREACH(uavo_list, obj) { + UAVO_LIST_ITERATE(obj) (*iterator) ((UAVObjHandle) obj); (*iterator) ((UAVObjHandle) &obj->metaObj); } diff --git a/flight/UAVObjects/uavobjecttemplate.c b/flight/UAVObjects/uavobjecttemplate.c index f8db8a024..d2e0e8f34 100644 --- a/flight/UAVObjects/uavobjecttemplate.c +++ b/flight/UAVObjects/uavobjecttemplate.c @@ -40,7 +40,7 @@ #include "$(NAMELC).h" // Private variables -static UAVObjHandle handle = NULL; +static UAVObjHandle handle __attribute__((section("_uavo_handles"))); /** * Initialize object. From f0fb22c090c4e448f6e8e0ff621c3b0ad986e28f Mon Sep 17 00:00:00 2001 From: Stacey Sheldon Date: Sun, 18 Nov 2012 16:48:13 -0500 Subject: [PATCH 4/4] uavobjectmanager: initialize new uavo_handles section The uavo_handles section is not part of the .data segment so it doesn't get automatically zeroed during the startup code. This led to random data being stored in the table which resulted in bus errors at runtime. This change ensures that the table is zeroed before we start to use it. --- flight/UAVObjects/uavobjectmanager.c | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/flight/UAVObjects/uavobjectmanager.c b/flight/UAVObjects/uavobjectmanager.c index 7938f99c3..cb46c36c0 100644 --- a/flight/UAVObjects/uavobjectmanager.c +++ b/flight/UAVObjects/uavobjectmanager.c @@ -42,12 +42,12 @@ #define SET_BITS(var, shift, value, mask) var = (var & ~(mask << shift)) | (value << shift); /* Table of UAVO handles registered at compile time */ -extern struct UAVOData * __start__uavo_handles __attribute__((weak)); -extern struct UAVOData * __stop__uavo_handles __attribute__((weak)); +extern struct UAVOData * __start__uavo_handles[] __attribute__((weak)); +extern struct UAVOData * __stop__uavo_handles[] __attribute__((weak)); #define UAVO_LIST_ITERATE(_item) \ -for (struct UAVOData ** _uavo_slot = &__start__uavo_handles; \ - _uavo_slot <= &__stop__uavo_handles; \ +for (struct UAVOData ** _uavo_slot = __start__uavo_handles; \ + _uavo_slot && _uavo_slot < __stop__uavo_handles; \ _uavo_slot++) { \ struct UAVOData * _item = *_uavo_slot; \ if (_item == NULL) continue; @@ -199,6 +199,10 @@ int32_t UAVObjInitialize() // Initialize variables memset(&stats, 0, sizeof(UAVObjStats)); + // Initialize the uavo handle table + memset(__start__uavo_handles, 0, + (uintptr_t)__stop__uavo_handles - (uintptr_t)__start__uavo_handles); + // Create mutex mutex = xSemaphoreCreateRecursiveMutex(); if (mutex == NULL)