goToMain / libosdp

Implementation of IEC 60839-11-5 OSDP (Open Supervised Device Protocol); provides a C library with support for C++, Rust and Python3
https://libosdp.sidcha.dev
Apache License 2.0
139 stars 73 forks source link

Permanent command ignored when temporary control code is 1 #200

Open elanto-dev opened 1 week ago

elanto-dev commented 1 week ago

I am using Python library to mock the PD device on Raspberry Pi in order to send the card information to the access control system and receive the commands (mainly LED and Buzzer). And I had troubles with green and red LED commands, since they have sent, seemingly, the same command. After further investigation, I have found that the LED command was processed as temporary LED command, while the information was written in the permanent part of the payload.

Describe the bug The LED command code from data.c (Python project) ignores permanent LED information, when the control code is set to 1. The source code bit from python/osdp_sys/data.c on lines 49-52:

if (cmd->led.temporary.control_code != 0) {
    p = &cmd->led.temporary;
    is_temporary = true;
}

However, when control code is set to 1, the LED permanent state should be displayed. With the following payload: 0000010000000000000101010202 I have the following when I print the commands:

Expected commands

{'command': 2, 'temporary': False, 'led_number': 0, 'reader': 0, 'control_code': 1, 'off_color': 2, 'on_color': 2, 'on_count': 1, 'off_count': 1}

Observed commands

{'command': 2, 'temporary': True, 'led_number': 0, 'reader': 0, 'control_code': 1, 'off_color': 0, 'on_color': 0, 'on_count': 0, 'off_count': 0, 'timer_count': 0}

I am not sure what the control command 2 does (I do not use it for the project), but when I modified the code above to:

if (cmd->led.temporary.control_code == 2) {
    p = &cmd->led.temporary;
    is_temporary = true;
}

I am able to read the permanent data sent to my PD.

P.S. I am very new to OSDP, but hope the explanation was clear. I can provide any additional information that can be legally shared if needed.

sidcha commented 1 week ago

@elanto-dev The temporary control code 1 is a request to cancel any other temporary LED activity that might be ongoing (from a previous command). You can read about it in the API documentation here or in the source code here.

Based on your description, I think your CP sending a permanent LED command and asking the PD to cancel any temporary LED activities. The python implementation has a limitation (undocumented) that it can pass on either temporary or permanent records only. When library sees the cancel-temporary request, it passes it on to the app which leads to the permanent records being ignored.

sidcha commented 1 week ago

Please check if this following workaround helps (In your permanent LED command, you should see an additional boolean cancel_temporary).

diff --git a/python/osdp_sys/data.c b/python/osdp_sys/data.c
index 75490aa..a62ba7e 100644
--- a/python/osdp_sys/data.c
+++ b/python/osdp_sys/data.c
@@ -43,10 +43,14 @@ static int pyosdp_make_struct_cmd_output(struct osdp_cmd *p, PyObject *dict)

 static int pyosdp_make_dict_cmd_led(PyObject *obj, struct osdp_cmd *cmd)
 {
+       bool cancel_temporary = false;
        bool is_temporary = false;
        struct osdp_cmd_led_params *p = &cmd->led.permanent;

-       if (cmd->led.temporary.control_code != 0) {
+       if (cmd->led.temporary.control_code == 1) {
+               cancel_temporary = true;
+       }
+       if (cmd->led.temporary.control_code == 2) {
                p = &cmd->led.temporary;
                is_temporary = true;
        }
@@ -66,8 +70,13 @@ static int pyosdp_make_dict_cmd_led(PyObject *obj, struct osdp_cmd *cmd)
                return -1;
        if (pyosdp_dict_add_int(obj, "off_count", p->off_count))
                return -1;
-       if (is_temporary && pyosdp_dict_add_int(obj, "timer_count", p->timer_count))
-               return -1;
+       if (is_temporary) {
+               if (pyosdp_dict_add_int(obj, "timer_count", p->timer_count))
+                       return -1;
+       } else {
+               if (pyosdp_dict_add_bool(obj, "cancel_temporary", cancel_temporary))
+                       return -1;
+       }
        return 0;
 }
elanto-dev commented 1 week ago

Good day @sidcha, the suggested code indeed does what I expect. Thank you for the help and explanation!