Skip to content

Conversation

@sgouraud
Copy link

According OSDP specification 2.2.2:

  • osdp_LED : The ON Time OFF Time values cannot both be set to zero.
  • osdp_BUZ : The ON duration of the sound, in units of 100 ms. Must be nonzero unless the tone code is 0x01 (off.)

This PR intends to check this and Nack if any of LED/Buzzer command reach this error condition

sidcha added a commit that referenced this pull request Oct 26, 2025
According OSDP specification 2.2.2:

 - osdp_LED : The ON Time OFF Time values cannot both be set to zero.
 - osdp_BUZ : The ON duration of the sound, in units of 100 ms. Must be nonzero unless the tone code is 0x01 (off.)

This patch adds a validation method which acts as a gate to the
application callback, thereby sending a NAK(9) to the CP if the
validation fails.

Related-to: #246
Co-Developed-By: Samuel Gouraud <sgouraud@fdi-access.com>
Signed-off-by: Siddharth Chandrasekaran <sidcha.dev@gmail.com>
@sidcha
Copy link
Member

sidcha commented Oct 26, 2025

@sgouraud Thanks for the PR. I merged it with a slight tweak to where it gets invoked; but should be functionally identical.

Closing this PR as merged.

@sidcha sidcha closed this Oct 26, 2025
@sgouraud
Copy link
Author

Hi @sidcha,
Thanks for the merged ! But as i understand your improvement, you made the check on OSDP_CMD_OUTPUT, instead of OSDP_CMD_BUZZER, is it normal? 😬

@sidcha
Copy link
Member

sidcha commented Oct 27, 2025

No, that was a typo. Nice catch, should be fixed now. Thanks.

@sgouraud
Copy link
Author

sgouraud commented Nov 3, 2025

Hi @sidcha,
I'm currently integrating your implementation into our code, but it looks like you're only checking the first led/buzzer command. From my understanding of the specification, OSDP_CMD_BUZZER and OSDP_CMD_LED can chain multiple commands together. Is it possible to check this?
Thanks in advance 🙂

@sidcha
Copy link
Member

sidcha commented Nov 4, 2025

For buzzer, we don't support multiple records within the same command; this seems to be a later change to the specification. I see SIA has been slowly introducing such non-backward compatible changes to the specification :(.

For LEDs, we do handle multiple records and each of them will be validated as expected.

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

Successfully merging this pull request may close these issues.

2 participants