NumS and NumV fields in Device Composition Data are doubled

[JonasJ]

Using Mesh SDK 3.2.0.

The returned Device Composition Data is malformed when requesting it after the node has been reset. This is because the sig_model_count and vendor_model_count for each element is incremented twice for each model.

The counter is incremented when the model is added to the access layer with access_model_add() but also when the models are restored from flash in restore_acquired_model(). Since the models are written to flash after a Config Node Reset message they will be recovered on the next boot causing all models to be counted twice.

The following patch fixes the issue.

diff --git a/mesh/access/src/access.c b/mesh/access/src/access.c
index e4f3ac0..401191e 100644
--- a/mesh/access/src/access.c
+++ b/mesh/access/src/access.c
@@ -804,7 +804,6 @@ static fm_iterate_action_t restore_acquired_model(const fm_entry_t * p_entry, vo
     }
 
     memcpy(&m_model_pool[index].model_info, p_model_data_entry, sizeof(access_model_state_data_t));
-    increment_model_count(p_model_data_entry->element_index, p_model_data_entry->model_id.company_id);
     ACCESS_INTERNAL_STATE_OUTDATED_CLR(m_model_pool[index].internal_state);
     return FM_ITERATE_ACTION_CONTINUE;
 }

Thanks.

[Edvin]

Hello,

Thank you for reporting. I have forwarded this to our Mesh team, and I will wait for their reply. 

I haven't done any testing with this fix myself, but did you also move the line:

ACCESS_INTERNAL_STATE_OUTDATED_CLR(m_model_pool[index].internal_state);

Best regards,

Edvin

[JonasJ]

Yes, I'm running with some other patches that fixes several issues with DSM and Access. See ticket https://devzone.nordicsemi.com/support/239135 (Access layer persist data to flash on every boot).

Thanks.

[Edvin]

Does this cause your application to fail at some point? Because it doesn't do that by default in the examples in SDK3.2.0, as far as I can tell. I don't know what other changes you did in access.c, but it doesn't look like restore_acquired_model is being called unless something wrong happens in restore_flash_data().

restore_aquired_models should only increment she model count if index < ACCESS_MODEL_COUNT:

static fm_iterate_action_t restore_acquired_model(const fm_entry_t * p_entry, void * p_success)
{
    access_model_state_data_t * p_model_data_entry = (access_model_state_data_t *) p_entry->data;
    uint16_t index = p_entry->header.handle & FLASH_HANDLE_TO_ACCESS_HANDLE_MASK;
    if (index >= ACCESS_MODEL_COUNT ||
        p_model_data_entry->element_index >= ACCESS_ELEMENT_COUNT ||
        (m_model_pool[index].model_info.subscription_pool_index < ACCESS_SUBSCRIPTION_LIST_COUNT &&
         (m_model_pool[index].model_info.subscription_pool_index != p_model_data_entry->subscription_pool_index ||
          !ACCESS_INTERNAL_STATE_IS_ALLOCATED(m_subscription_list_pool[p_model_data_entry->subscription_pool_index].internal_state))))
    {
        /* Fail if:
         * - The model handle index is invalid
         * - The element index pointed to is invalid
         * - The model has an allocated subscription list, but the index doesn't match the one in
         *   flash or isn't allocated in the subscription list pool
         *
         * All of these conditions are signs that the user access init code has changed (order of
         * allocation etc.)
         */
        *((bool *) p_success) = false;
        return FM_ITERATE_ACTION_STOP;
    }

    if (p_model_data_entry->subscription_pool_index < ACCESS_SUBSCRIPTION_LIST_COUNT)
    {
        ACCESS_INTERNAL_STATE_RESTORED_SET(m_subscription_list_pool[p_model_data_entry->subscription_pool_index].internal_state);
    }
    memcpy(&m_model_pool[index].model_info, p_model_data_entry, sizeof(access_model_state_data_t));
    increment_model_count(p_model_data_entry->element_index, p_model_data_entry->model_id.company_id);
    return FM_ITERATE_ACTION_CONTINUE;
}

Because if it returns FM_ITERATE_ACTION_STOP, then restore_flash_data will return 0, and access_models_increment will not be called in restore_acquired_model().

[JonasJ]

Hi,

FM_ITERATE_ACTION_STOP will be returned if the content on the flash is consider broken, see the comment.

There is nothing wrong with the flash content thus increment_model_count() is called and FM_ITERATE_ACTION_CONTINUE is returned.

As I mentioned in the initial report, increment_model_count() is also called in access_model_add(). Thus, the counter is incremented in two places. When the model is added by the application code and when it's restored from the flash.

Thanks.

[JonasJ]

This will cause problems for apps that actually validate the information sent by the node and simply reject broken messages.