Simple_On_Off_Model callbacks are called multiple times each message

[Heiner]

Hi,


so I witnessed this weird phenomenon:

When I use the simple On-Off-Model (as a base for a custom vendor model) in the switch application, the handle_set function in simple on_off_server. and therefore the callback function in main.c are called tree times.

As far as my guesses go, this is caused by the access layer calling the handle multiple times, but I'm really not sure.

Debugging didn't quite help yet, since the debugger exits the c-code after the callback function and does not return to the previous function that has called the handler in the first place.



This is not a crucial error, but still it would be quite interesting to know what is causing this behavior.



I'm currently still using the v3.1 of the mesh SDK.

Thank you,

Heiner

[Joakim Jakobsen]

Hi. 

Talked to one of my colleagues about this. 

The reason that the command is sent three times is could be because you don't reply to the client. The client expects that the set command is ACKED, otherwise it will retry 3 times. 

You could try to use set_unack instead, that way the client won't retry three times when it doesn't receive an ACK. 

Cheers, 
Joakim

[Heiner]

Hi Joakim,

Joakim Jakobsen said:

You could try to use set_unack instead

this is weird, because I actually used simple_on_off_client_set_unreliable for my messages.

The differences I could find so far, where that simple_on_off_client_set_unreliable uses an argument called repeats.

It would make sense if that is the cause, because repeats is set to 2. However if I set the repeat counter to 0 I cannot seem to transmit any message. (When I set repeats to 1 the handler is called two times instead of tree times.)

From my point of view the function handle_set_unreliable_cb, unlike handle_set_cb, does not send a reply status to the access_model. 

Which makes sense, since it's set unacknowledged.

But this leaves me with new questions, like:

How can I set simple_on_off_client_set_unreliable to send the message only once?

Or is there maybe something that needs to be acknowledged with handle_set_unreliable_cb (at least on a lower level)?

It would be great if you could look into that again.

Best,

Heiner

[Joakim Jakobsen]

Ok, then I don' think it's related to my previous answer. 

When you added the simple on-off model, did you follow our guide in the infocenter?
(Creating new models)

Could it be possible for you to upload your project so that I can look at your code?
If you don't want to share your code in the public forum, we can turn the ticket private first. 

Best regards, 
Joakim

[Heiner]

Hi Joakim,

I followed your guide and tried to stick as close as possible to it.

Joakim Jakobsen said:

If you don't want to share your code in the public forum, we can turn the ticket private first. 

There aren't too many changes, at least not yet. So I think it should be fine if we leave it in the public forum for now.

As mentioned I used your simple_on_off model as a base for my code.

model common.h

/* Copyright (c) 2010 - 2018, Nordic Semiconductor ASA
 * All rights reserved.
 *
 * Redistribution and use in source and binary forms, with or without modification,
 * are permitted provided that the following conditions are met:
 *
 * 1. Redistributions of source code must retain the above copyright notice, this
 * list of conditions and the following disclaimer.
 *
 * 2. Redistributions in binary form, except as embedded into a Nordic
 *    Semiconductor ASA integrated circuit in a product or a software update for
 *    such product, must reproduce the above copyright notice, this list of
 *    conditions and the following disclaimer in the documentation and/or other
 *    materials provided with the distribution.
 *
 * 3. Neither the name of Nordic Semiconductor ASA nor the names of its
 *    contributors may be used to endorse or promote products derived from this
 *    software without specific prior written permission.
 *
 * 4. This software, with or without modification, must only be used with a
 *    Nordic Semiconductor ASA integrated circuit.
 *
 * 5. Any software provided in binary form under this license must not be reverse
 *    engineered, decompiled, modified and/or disassembled.
 *
 * THIS SOFTWARE IS PROVIDED BY NORDIC SEMICONDUCTOR ASA "AS IS" AND ANY EXPRESS
 * OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES
 * OF MERCHANTABILITY, NONINFRINGEMENT, AND FITNESS FOR A PARTICULAR PURPOSE ARE
 * DISCLAIMED. IN NO EVENT SHALL NORDIC SEMICONDUCTOR ASA OR CONTRIBUTORS BE
 * LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
 * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE
 * GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION)
 * HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT
 * LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT
 * OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 */

#ifndef SIMPLE_ON_OFF_COMMON_H__
#define SIMPLE_ON_OFF_COMMON_H__

#include <stdint.h>
#include "access.h"

/**
 * @defgroup SIMPLE_ON_OFF_MODEL Simple OnOff model
 * This model implements the message based interface required to
 * set the 1 bit value on the server.
 *
 * This implementation of a simple OnOff model can be used to switch things
 * on or off by manipulating a single on/off state. The intention of this model
 * is to have a simple example model that can be used as a baseline for constructing
 * your own model.
 *
 * Do not confuse the simple OnOff model with the Generic OnOff Model specified
 * in the Mesh Model Specification v1.0. The Generic OnOff Model provides additional
 * features such as control over when and for how long the transition between
 * the on/off state should be performed.
 *
 * @note When the server has a publish address set (as in the light switch example),
 * the server will publish its state to its publish address every time its state changes.
 *
 * For more information about creating models, see
 * @ref md_doc_getting_started_how_to_models.
 *
 * Model Identification
 * @par
 * Company ID: @ref SIMPLE_ON_OFF_COMPANY_ID
 * @par
 * Simple OnOff Client Model ID: @ref SIMPLE_ON_OFF_CLIENT_MODEL_ID
 * @par
 * Simple OnOff Server Model ID: @ref SIMPLE_ON_OFF_SERVER_MODEL_ID
 *
 * List of supported messages:
 * @par
 * @copydoc SIMPLE_ON_OFF_OPCODE_SET
 * @par
 * @copydoc SIMPLE_ON_OFF_OPCODE_GET
 * @par
 * @copydoc SIMPLE_ON_OFF_OPCODE_SET_UNRELIABLE
 * @par
 * @copydoc SIMPLE_ON_OFF_OPCODE_STATUS
 *
 * @ingroup MESH_API_GROUP_VENDOR_MODELS
 * @{
 * @defgroup SIMPLE_ON_OFF_COMMON Common Simple OnOff definitions
 * Types and definitions shared between the two Simple OnOff models.
 * @{
 */


/** Vendor specific company ID for Simple OnOff model */
#define SIMPLE_ON_OFF_COMPANY_ID    (ACCESS_COMPANY_ID_NORDIC)

/** Simple OnOff opcodes. */
typedef enum
{
    SIMPLE_ON_OFF_OPCODE_SET = 0xC1,            /**< Simple OnOff Acknowledged Set. */
    SIMPLE_ON_OFF_OPCODE_GET = 0xC2,            /**< Simple OnOff Get. */
    SIMPLE_ON_OFF_OPCODE_SET_UNRELIABLE = 0xC3, /**< Simple OnOff Set Unreliable. */
    SIMPLE_ON_OFF_OPCODE_STATUS = 0xC4,         /**< Simple OnOff Status. */
	SIMPLE_ON_OFF_OPCODE_SET_FRIEND = 0xC5,		/** Heiner Set Adress of Friend Node */
} simple_on_off_opcode_t;

/** Message format for the Simple OnOff Set message. */
typedef struct __attribute((packed))
{
    uint8_t state; /** Heiner: State 1 */
		/** Heiner: State 2 hier hinzufügen*/
    uint8_t tid;    /**< Transaction number. */
} simple_on_off_msg_set_t;

/** Message format for th Simple OnOff Set Unreliable message. */
typedef struct __attribute((packed))
{
    uint8_t state; /** Heiner: State 1  */
		/** Heiner: State 2 hier hinzufügen*/
	uint8_t tid;    /**< Transaction number. */
} simple_on_off_msg_set_unreliable_t;

/** Message format for the Simple OnOff Status message. */
typedef struct __attribute((packed))
{
    uint8_t present_state; /**< Current state. */ // Heiner
} simple_on_off_msg_status_t;

//** Heiner: format for Simple OnOff Friend message */
typedef struct __attribute((packed))
{
    uint16_t address; /** Heiner: Adresse des FRND Knoten */
    uint8_t tid;    /**< Transaction number. */
} simple_on_off_msg_set_friend_t;


/** @} end of SIMPLE_ON_OFF_COMMON */
/** @} end of SIMPLE_ON_OFF_MODEL */
#endif /* SIMPLE_ON_OFF_COMMON_H__ */

model server.c

/* Copyright (c) 2010 - 2018, Nordic Semiconductor ASA
 * All rights reserved.
 *
 * Redistribution and use in source and binary forms, with or without modification,
 * are permitted provided that the following conditions are met:
 *
 * 1. Redistributions of source code must retain the above copyright notice, this
 * list of conditions and the following disclaimer.
 *
 * 2. Redistributions in binary form, except as embedded into a Nordic
 *    Semiconductor ASA integrated circuit in a product or a software update for
 *    such product, must reproduce the above copyright notice, this list of
 *    conditions and the following disclaimer in the documentation and/or other
 *    materials provided with the distribution.
 *
 * 3. Neither the name of Nordic Semiconductor ASA nor the names of its
 *    contributors may be used to endorse or promote products derived from this
 *    software without specific prior written permission.
 *
 * 4. This software, with or without modification, must only be used with a
 *    Nordic Semiconductor ASA integrated circuit.
 *
 * 5. Any software provided in binary form under this license must not be reverse
 *    engineered, decompiled, modified and/or disassembled.
 *
 * THIS SOFTWARE IS PROVIDED BY NORDIC SEMICONDUCTOR ASA "AS IS" AND ANY EXPRESS
 * OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES
 * OF MERCHANTABILITY, NONINFRINGEMENT, AND FITNESS FOR A PARTICULAR PURPOSE ARE
 * DISCLAIMED. IN NO EVENT SHALL NORDIC SEMICONDUCTOR ASA OR CONTRIBUTORS BE
 * LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
 * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE
 * GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION)
 * HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT
 * LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT
 * OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 */

#include "simple_on_off_server.h"
#include "simple_on_off_common.h"

#include <stdint.h>
#include <stddef.h>

#include "access.h"
#include "nrf_mesh_assert.h"
#include "log.h"

/*****************************************************************************
 * Static functions
 *****************************************************************************/

static void reply_status(const simple_on_off_server_t * p_server,
                         const access_message_rx_t * p_message,
//                         bool present_on_off) 
                         uint8_t present_state)
{
    simple_on_off_msg_status_t status;
//    status.present_on_off = present_on_off ? 1 : 0;
    status.present_state; // Heiner
    access_message_tx_t reply;
    reply.opcode.opcode = SIMPLE_ON_OFF_OPCODE_STATUS;
    reply.opcode.company_id = SIMPLE_ON_OFF_COMPANY_ID;
    reply.p_buffer = (const uint8_t *) &status;
    reply.length = sizeof(status);
    reply.force_segmented = false;
    reply.transmic_size = NRF_MESH_TRANSMIC_SIZE_DEFAULT;
    reply.access_token = nrf_mesh_unique_token_get();

    (void) access_model_reply(p_server->model_handle, p_message, &reply);
}

/*****************************************************************************
 * Opcode handler callbacks
 *****************************************************************************/

static void handle_set_cb(access_model_handle_t handle, const access_message_rx_t * p_message, void * p_args)
{
    simple_on_off_server_t * p_server = p_args;
    NRF_MESH_ASSERT(p_server->set_cb != NULL);

    // bool value = (((simple_on_off_msg_set_t*) p_message->p_data)->on_off) > 0; 
    uint8_t value = (((simple_on_off_msg_set_t*) p_message->p_data)->state);//Heiner value -> uint8
    value = p_server->set_cb(p_server, value);
    reply_status(p_server, p_message, value);
    (void) simple_on_off_server_status_publish(p_server, value); /* We don't care about status */
}

static void handle_get_cb(access_model_handle_t handle, const access_message_rx_t * p_message, void * p_args)
{
    simple_on_off_server_t * p_server = p_args;
    NRF_MESH_ASSERT(p_server->get_cb != NULL);
    reply_status(p_server, p_message, p_server->get_cb(p_server));
}

static void handle_set_unreliable_cb(access_model_handle_t handle, const access_message_rx_t * p_message, void * p_args)
{
    simple_on_off_server_t * p_server = p_args;
    NRF_MESH_ASSERT(p_server->set_cb != NULL);
//    bool value = (((simple_on_off_msg_set_unreliable_t*) p_message->p_data)->on_off) > 0;
	uint8_t value = (((simple_on_off_msg_set_unreliable_t*) p_message->p_data)->state);//Heiner value -> uint8
    value = p_server->set_cb(p_server, value);
    (void) simple_on_off_server_status_publish(p_server, value);
}

// Heiner
static void handle_set_friend_cb(access_model_handle_t handle, const access_message_rx_t * p_message, void * p_args)
{
    simple_on_off_server_t * p_server = p_args;
    NRF_MESH_ASSERT(p_server->friend_cb != NULL);
	uint16_t value = (((simple_on_off_msg_set_friend_t*) p_message->p_data)->address);
    value = p_server->friend_cb(p_server, value);
    //(void) simple_on_off_server_status_publish(p_server, value); // Heiner publish Friend Adress if necessary
}

static const access_opcode_handler_t m_opcode_handlers[] =
{
    {ACCESS_OPCODE_VENDOR(SIMPLE_ON_OFF_OPCODE_SET,            SIMPLE_ON_OFF_COMPANY_ID), handle_set_cb},
    {ACCESS_OPCODE_VENDOR(SIMPLE_ON_OFF_OPCODE_GET,            SIMPLE_ON_OFF_COMPANY_ID), handle_get_cb},
    {ACCESS_OPCODE_VENDOR(SIMPLE_ON_OFF_OPCODE_SET_UNRELIABLE, SIMPLE_ON_OFF_COMPANY_ID), handle_set_unreliable_cb},
    {ACCESS_OPCODE_VENDOR(SIMPLE_ON_OFF_OPCODE_SET_FRIEND,     SIMPLE_ON_OFF_COMPANY_ID), handle_set_friend_cb}
};

/*****************************************************************************
 * Public API
 *****************************************************************************/

uint32_t simple_on_off_server_init(simple_on_off_server_t * p_server, uint16_t element_index)
{
    if (p_server == NULL ||
        p_server->get_cb == NULL ||
        p_server->set_cb == NULL)
    {
        return NRF_ERROR_NULL;
    }

    access_model_add_params_t init_params;
    init_params.element_index =  element_index;
    init_params.model_id.model_id = SIMPLE_ON_OFF_SERVER_MODEL_ID;
    init_params.model_id.company_id = SIMPLE_ON_OFF_COMPANY_ID;
    init_params.p_opcode_handlers = &m_opcode_handlers[0];
    init_params.opcode_count = sizeof(m_opcode_handlers) / sizeof(m_opcode_handlers[0]);
    init_params.p_args = p_server;
    init_params.publish_timeout_cb = NULL;
    return access_model_add(&init_params, &p_server->model_handle);
}

//Heiner value -> uint8
uint32_t simple_on_off_server_status_publish(simple_on_off_server_t * p_server, uint8_t value) 
{
    simple_on_off_msg_status_t status;
//    status.present_on_off = value ? 1 : 0;
    status.present_state = value; // Heiner
    access_message_tx_t msg;
    msg.opcode.opcode = SIMPLE_ON_OFF_OPCODE_STATUS;
    msg.opcode.company_id = SIMPLE_ON_OFF_COMPANY_ID;
    msg.p_buffer = (const uint8_t *) &status;
    msg.length = sizeof(status);
    msg.force_segmented = false;
    msg.transmic_size = NRF_MESH_TRANSMIC_SIZE_DEFAULT;
    msg.access_token = nrf_mesh_unique_token_get();
    return access_model_publish(p_server->model_handle, &msg);
}

While uploading the code I took a closer look at the set_unreliable function from server.c

uint32_t simple_on_off_client_set_unreliable(simple_on_off_client_t * p_client, uint8_t state, uint8_t repeats) // Heiner
{
    simple_on_off_msg_set_unreliable_t set_unreliable;
//    set_unreliable.on_off = on_off ? 1 : 0;
    set_unreliable.state = state; // Heiner
    set_unreliable.tid = m_tid++;

    access_message_tx_t message;
    message.opcode.opcode = SIMPLE_ON_OFF_OPCODE_SET_UNRELIABLE;
    message.opcode.company_id = SIMPLE_ON_OFF_COMPANY_ID;
    message.p_buffer = (const uint8_t*) &set_unreliable;
    message.length = sizeof(set_unreliable);
    message.force_segmented = false;
    message.transmic_size = NRF_MESH_TRANSMIC_SIZE_DEFAULT;

    uint32_t status = NRF_SUCCESS;
    for (uint8_t i = 0; i < repeats; ++i)
    {
        message.access_token = nrf_mesh_unique_token_get();
        status = access_model_publish(p_client->model_handle, &message);
        if (status != NRF_SUCCESS)
        {
            break;
        }
    }
    return status;
}

This explains why I couldn't get any message through before.

Heiner said:

if I set the repeat counter to 0 I cannot seem to transmit any message.

However if set the repeat counter to 1 the message should be transmitted just once, but the callback is still being called twice. So the root of the problem must be somewhere else.

Heiner said:

When I set repeats to 1 the handler is called two times instead of tree times.

Best,

Heiner

[Joakim Jakobsen]

Hi. 

Thanks for the detailed response and the code-upload. 

I can't seem to find the root cause for this. 

Could you double check that the Public Retransmit Count (config_publication_state_t::retransmit_count) is set to zero?

Regards, 
Joakim

[Joakim Jakobsen]

Hi. 

Thanks for the detailed response and the code-upload. 

I can't seem to find the root cause for this. 

Could you double check that the Public Retransmit Count (config_publication_state_t::retransmit_count) is set to zero?

Regards, 
Joakim