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
130 stars 69 forks source link

OSDP_CMD_ABORT is not exposed to the application. #97

Closed surendersampath closed 1 year ago

surendersampath commented 1 year ago

Describe the bug CMD_ABORT's command ID was fixed in issue #93 . However, this is not being exposed to the application. Hence, not able to invoke abort command via osdp_cp_send_command() . image

Expected behavior CMD_ABORT to be exposed to application.

Observed behavior CMD_ABORT is not exposed to application.

sidcha commented 1 year ago

@surendersampath, the OSDP defined command CMD_ABORT is not meant for external use by applications, libosdp abstracts those details and provides its own high level commands though enum osdp_cmd_e.

Something is missing though: the file transfer API osdp_file_tx_abort should send CMD_ABORT to the peer before reseting the state. Do you have any other use case for why ABORT needs to be exposed to the application?

surendersampath commented 1 year ago

I would like to use the abort command to stop the ongoing file transfer. But, Do you suggest that just calling file_tx_abort() function is sufficient?

Sent from Outlook for iOShttps://aka.ms/o0ukef


From: Siddharth Chandrasekaran @.> Sent: Thursday, December 8, 2022 6:47:26 PM To: goToMain/libosdp @.> Cc: Surender Sampath @.>; Mention @.> Subject: Re: [goToMain/libosdp] OSDP_CMD_ABORT is not exposed to the application. (Issue #97)

@surendersampathhttps://github.com/surendersampath, the OSDP defined command CMD_ABORT is not meant for external use by applications, libosdp abstracts those details and provides its own high level commands though enum osdp_cmd_e.

Something is missing though: the file transfer API osdp_file_tx_abort should send CMD_ABORT to the peer before reseting the state. Do you have any other use case for why ABORT needs to be exposed to the application?

— Reply to this email directly, view it on GitHubhttps://github.com/goToMain/libosdp/issues/97#issuecomment-1343179798, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AKQR6JQ67ULXKZ4AUVL6W73WMIUL5ANCNFSM6AAAAAASYLFCYE. You are receiving this because you were mentioned.Message ID: @.***>

sidcha commented 1 year ago

No, I spoke too soon, osdp_file_tx_abort is an internal method and serves a different purpose.

After some inspection, I think it more sensible to abort an ongoing transfer using the existing OSDP_CMD_FILE_TX command's flags (osdp_cmd_file_tx::flags) field. Will make that change.

surendersampath commented 1 year ago

Another observation from me is that when the file tx build command fails, it sets the phy state to be offline. I noticed this while sending an abort command which may not be the suitable action to abort a file transfer. But it'd be helpful if you could what's the best way to abort an on going file transfer?

Sent from Outlook for iOShttps://aka.ms/o0ukef


From: Siddharth Chandrasekaran @.> Sent: Thursday, December 8, 2022 7:03:54 PM To: goToMain/libosdp @.> Cc: Surender Sampath @.>; Mention @.> Subject: Re: [goToMain/libosdp] OSDP_CMD_ABORT is not exposed to the application. (Issue #97)

No, I spoke too soon, osdp_file_tx_abort is an internal method and servers a different purpose.

After some inspection, I think it more sensible to abort an ongoing transfer using the existing OSDP_CMD_FILE_TX command's flags (osdp_cmd_file_tx::flags) field. Will make that change.

— Reply to this email directly, view it on GitHubhttps://github.com/goToMain/libosdp/issues/97#issuecomment-1343196614, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AKQR6JWBSSG7CQRN3AROU3DWMIWJVANCNFSM6AAAAAASYLFCYE. You are receiving this because you were mentioned.Message ID: @.***>

sidcha commented 1 year ago

@surendersampath, You can use the newly introduced bit OSDP_CMD_FILE_TX_FLAG_CANCEL in osdp_cmd_file_tx::flags to abort an ongoing transfer. PD will not receive this command directly (as with other commands); instead the CP will send OSDP command CMD_ABORT to PD to signal the premature termination of current file transfer.

There is also a test case to ensure its correctness, so I will close this issue. For the command build failure, I'll create a new issue to track it.