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
137 stars 71 forks source link

TX_Decode failing for File Transfer Abort at PD. #100

Closed surendersampath closed 1 year ago

surendersampath commented 1 year ago

Describe the bug PD does not handle the incoming FILE TX command from CP with flag set as OSDP_CMD_FILE_TX_FLAG_CANCEL . Decode fails as the length is lesser than expected. -------------At PD. image

-------------At CP. image

Expected behavior PD to handle the the file transfer command with OSDP_CMD_FILE_TX_FLAG_CANCEL . Observed behavior A description of what happened.

Additional context I'm using the below code snipped to abort an on-going file transfer.

// Function to Abort File Transfer

bool xAbortTransfer(eOsdpFileTypes xFileType)
{
    bool xStatus = false;
    struct osdp_cmd xAbortCommand;
    xAbortCommand.id = OSDP_CMD_FILE_TX;
    xAbortCommand.file_tx.id = xFileType;
    xAbortCommand.file_tx.flags = OSDP_CMD_FILE_TX_FLAG_CANCEL;    
    xStatus = ( osdp_cp_send_command( ctx, 0, &xAbortCommand ) == 0 ) ? true : false;

    return xStatus;
}

Additional context

Sequence is :

  1. The sequence happens such that send the first file with signature and wait for it to complete. Then Starts sending the second file (binary).
  2. The signature is cached in the RAM while the binary is stored onto the flash memory. This is by decided by the file ID.
  3. During the transmission of second file, the CP wants to abort an on going-transfer.
  4. CP sends the file TX command as you suggested in libosdp.
  5. PD sends an NAK due to mismatch in length as shown in the screenshots. It does not reset the file state here.
  6. CP starts polling PD again.
  7. Step 1 is initiated again by the CP to perform an Software update. However, PD still thinks transfer for Second file is still on going or OSDP_FILE_INPROG.

Suggested FIX : Add file_state_reset(f); in the TX_Decode if any error(such as length mistmatch) occurs?

sidcha commented 1 year ago

PD does not handle the incoming FILE TX command from CP with flag set as OSDP_CMD_FILE_TX_FLAG_CANCEL.

OSDP_CMD_FILE_TX_FLAG_CANCEL is not sent to the PD, it is consumed by CP to make the decision of sending the ABORT command. When the CP receives a command that has the cancel bit set, and no transfer is in progress (or the file ID does not match) we fail the command!

So, I suspect something else is causing the failure. What do you need to do to reproduce this issue? The best thing to do is to write a unit test that can reproduce this issue (everything needed to write such a test can be found here).

You can also generate CP and PD logs with packet trace enabled and log level set to debug. I can try to take a look and figure out what is going wrong.

Suggested FIX : Add file_state_reset(f); in the TX_Decode if any error(such as length mistmatch) occurs?

Since the failure is between LibOSDP CP <==> PD, I'd prefer to root cause and fix the bug properly rather than mask the issue by fixing the symptom with file_state_reset().

surendersampath commented 1 year ago

Thanks for looking into this. I'll try and re-create this issue using your unit test set up.

sidcha commented 1 year ago

@surendersampath, I have a potential fix for this issue. For now, I'm closing this issue but feel free to re-open if you can still reproduce it.