Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Does bacnet have a cov subscription recovery strategy? #477

Open
Twillpower opened this issue Aug 25, 2023 · 10 comments
Open

Does bacnet have a cov subscription recovery strategy? #477

Twillpower opened this issue Aug 25, 2023 · 10 comments

Comments

@Twillpower
Copy link

If device A initiates a permanent subscription to device B, but device B fails and restarts, how does device A know that it needs to re-initiate the subscription? Does device A need to delete the previous subscription, and if so, how to delete it?

@skarg
Copy link
Collaborator

skarg commented Aug 25, 2023

COV subscriptions with indefinite lifetimes (aka permanent) are discouraged. See Q/A 7.6 in the BTL Implementation Guildelines

7.6 Do not subscribe for COV notifications with 'Lifetime' set to zero (indefinite lifetime)
Devices should not issue SubscribeCOV or SubscribeCOVProperty requests with the 'Lifetime' parameter
set to zero. (The value zero is prohibited for SubscribeCOVProperty requests.) The COV subscription list
is not guaranteed to remain through a device reset or power loss, or the device holding the subscription
could fail and be replaced. Worse, if the subscription list is permanently held and the subscribing device is
removed, the storage for the removed device's subscription remains allocated forever.

SubscribeCOV requests should be issued with a non-zero 'Lifetime' parameter for some period (such as the
amount of time it is acceptable for the subscribing device to operate with out-of-date data), and the
subscription should be refreshed periodically.

Subscriptions include a subscriberProcessIdentifier that is unique per subscribing device. Any subsequent subscriptions to the same monitored object will replace the existing subscriptions with matching process-id and device source address. Example code from the library:

        if (COV_Subscriptions[index].flag.valid) {
            dest = cov_address_get(COV_Subscriptions[index].dest_index);
            if (dest) {
                address_match = bacnet_address_same(src, dest);
            } else {
                /* skip address matching - we don't have an address */
                address_match = true;
            }
            if ((COV_Subscriptions[index].monitoredObjectIdentifier.type ==
                    cov_data->monitoredObjectIdentifier.type) &&
                (COV_Subscriptions[index].monitoredObjectIdentifier.instance ==
                    cov_data->monitoredObjectIdentifier.instance) &&
                (COV_Subscriptions[index].subscriberProcessIdentifier ==
                    cov_data->subscriberProcessIdentifier) &&
                address_match) {
                existing_entry = true;

The SubscribeCOV services also includes a "cancellationRequest" option to delete the subscription.

@Twillpower
Copy link
Author

thank you, sir

@Twillpower
Copy link
Author

typedef struct BACnet_Subscribe_COV_Data {
    uint32_t subscriberProcessIdentifier;
    BACNET_OBJECT_ID monitoredObjectIdentifier;
    bool cancellationRequest;   /* true if this is a cancellation request */
    bool issueConfirmedNotifications;   /* optional */
    uint32_t lifetime;  /* seconds, optional */
    bool covSubscribeToProperty;  /* true to use per-property subscription */
    BACNET_PROPERTY_REFERENCE monitoredProperty;
    bool covIncrementPresent;   /* true if present */
    float covIncrement; /* optional */
    BACNET_ERROR_CLASS error_class;
    BACNET_ERROR_CODE error_code;
    struct BACnet_Subscribe_COV_Data *next;
} BACNET_SUBSCRIBE_COV_DATA;

Is this lifetime in seconds? I set this value to 300, 600, and 6000, and it is difficult to receive a notify. The value I tested is about 3s to send a notify.
After I set the value of lifetime to 149130 * 60, I can receive notify within a period of time.
The following are the two notify logs received next to each other, and the actual time interval between them is not 905048 - 731141 seconds.

CCOV: Received Notification!
CCOV: PID=331 instance=331 WY_OBJECT_HBD_LAMP 0 time remaining=905048 seconds 
CCOV: proprietary 513  current value 0 
CCOV: Sending Simple Ack!
CCOV: Received Notification!
CCOV: PID=331 instance=331 WY_OBJECT_HBD_LAMP 0 time remaining=731141 seconds 
CCOV: proprietary 513  current value 1 
CCOV: Sending Simple Ack!

@skarg
Copy link
Collaborator

skarg commented Sep 9, 2023

The handler_cov_timer_seconds() is expecting to have a number of elapsed seconds called during a main loop or task or thread:

/** Handler to check the list of subscribed objects for any that have changed
 *  and so need to have notifications sent.
 * @ingroup DSCOV
 * This handler will be invoked by the main program every second or so.
 * This example only handles Binary Inputs, but can be easily extended to
 * support other types.
 * For each subscribed object,
 *  - See if the subscription has timed out
 *    - Remove it if it has timed out.
 *  - See if the subscribed object instance has changed
 *    (eg, check with Binary_Input_Change_Of_Value() )
 *  - If changed,
 *    - Clear the COV (eg, Binary_Input_Change_Of_Value_Clear() )
 *    - Send the notice with cov_send_request()
 *      - Will be confirmed or unconfirmed, as per the subscription.
 *
 * @note worst case tasking: MS/TP with the ability to send only
 *        one notification per task cycle.
 *
 * @param elapsed_seconds [in] How many seconds have elapsed since last called.
 */
void handler_cov_timer_seconds(uint32_t elapsed_seconds)

@Twillpower
Copy link
Author

Thank you, I understand, but the parameter of this function is uint32_t, and the minimum value is 1. If the status changes, but the notification is sent after 1s, the delay will definitely be greater than 1s. This will affect real-time performance.

@skarg
Copy link
Collaborator

skarg commented Sep 13, 2023

handler_cov_timer_seconds() is only handling the timeout of the subscription, which doesn't affect COV Notification speed.

handler_cov_fsm() or handler_cov_task() is the function that handles the COV notifications and can be called as fast as the CPU allows. You can rewrite the h_cov.c if you require to have better event driven performance for your system - it is just an example of basic COV functionality.

@Twillpower
Copy link
Author

void my_task()
{
    while(1)
    {
        handler_cov_timer_seconds(1);
        wy_is_people = wy_get_is_people();
        HBD_LAMP_Is_People_Set(0, wy_is_people);
        vTaskDelay(10 / portTICK_PERIOD_MS);
        // or  vTaskDelay(1000 / portTICK_PERIOD_MS);  ?
    }
}

emmm, what I mean is probably similar to this. If you let it delay 1s, wy_is_people will be monitored once every second. If you let it delay 10ms, the monitoring will be more timely, but handler_cov_timer_seconds is not 1s from the last call. (Am I understanding it wrong, or is there a better solution)

@skarg
Copy link
Collaborator

skarg commented Sep 18, 2023

Where does your application call handler_cov_fsm() or handler_cov_task() ?

For example, using FreeRTOS, I typically run a BACnet Task at 1ms:

/**
 * @brief FreeRTOS main loop recurring task
 */
static void BACnet_Task(void *pvParameters)
{
    const portTickType xBlockTime =
        BACNET_TASK_MILLISECONDS / portTICK_RATE_MS;

    for (;;) {
        /* block to wait for a notification from either MS/TP task */
        xSemaphoreTake(BACnet_PDU_Available, xBlockTime);
        bacnet_task();
        BACnet_Task_High_Water_Mark = uxTaskGetStackHighWaterMark(NULL);
    }
    /* Should never go there */
    vTaskDelete(NULL);
}

then I have the BACnet Tasks like this:

void bacnet_task(void)
{
    ...
    if (mstimer_expired(&COV_Timer)) {
        mstimer_reset(&COV_Timer);
        handler_cov_timer_seconds(mstimer_interval(&COV_Timer)/1000);
    }
    handler_cov_fsm();
    ...
}

@Twillpower
Copy link
Author

Thanks, I roughly got it, I'm building cov into a separate task now.

void wy_updata_cov(void)
{
    AIR_INFO m_air_info_data = {0};
    struct mstimer COV_Timer;
    mstimer_set(&COV_Timer, 1 * 1000);
    while(1)
    {
    if (mstimer_expired(&COV_Timer)) {
        mstimer_reset(&COV_Timer);
        handler_cov_timer_seconds(mstimer_interval(&COV_Timer)/1000);
    }
        m_air_info_data = wy_get_air_info();
        AIR_Info_Set(0, m_air_info_data);
        handler_cov_task();
        vTaskDelay(10 / portTICK_PERIOD_MS);
    }
}

@BACnetEd
Copy link
Collaborator

BACnetEd commented Sep 20, 2023 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants