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

Add optional void* argument to osdp_set_command_complete_callback #89

Closed ejverat closed 1 year ago

ejverat commented 1 year ago

I've been using the library in C++ using OOP approach, but I faced a problem to deal with the command_complete_callback_t type, it can't be a member function pointer.

So, to solve this I want to propose to add an extra void* arg to osdp_set_command_complete_callback, so at this way I can give the object instance ("this" pointer) to the callback and then I will be able to access to class members inside the callback function,

So, the function pointer will be something like this typedef void (*osdp_command_complete_callback_t)(void* arg, int id) and function signature to set it would be void osdp_set_command_complete_callback(osdp_t *ctx, osdp_command_complete_callback_t cb, void* arg)

Also, we'll need to add the argument variable in the osdp struct.

sidcha commented 1 year ago

That sounds reasonable. And you have described everything needed to be done to support it (except that the python bindings also need to be updated). Would you like to create a PR for it? If not I'll get around to it soon-ish.

PS: this kind of change requests are easier described as a PR, you'd spend almost the same time making the change as you did describing it :)

ejverat commented 1 year ago

That sounds reasonable. And you have described everything needed to be done to support it (except that the python bindings also need to be updated). Would you like to create a PR for it? If not I'll get around to it soon-ish.

PS: this kind of change requests are easier described as a PR, you'd spend almost the same time making the change as you did describing it :)

I'll work on it, I need to fix this to integrate it in my project.

ejverat commented 1 year ago

Sorry I clicked wrong button, Is there an special rules I need to follow for PR? Should I create a branch from master branch? Anyways, I'm working in a fork.

sidcha commented 1 year ago

You can create a PR from any branch in your fork, just be sure to create the branch from the most recent master here (or rebase to it). Once you have the branch, make the change, commit (see how other commit messages are formatted) and push it to your fork. Then when you visit your forked repo URL, GitHub will prompt you to create a PR for that branch.

ejverat commented 1 year ago

You can create a PR from any branch in your fork, just be sure to create the branch from the most recent master here (or rebase to it). Once you have the branch, make the change, commit (see how other commit messages are formatted) and push it to your fork. Then when you visit your forked repo URL, GitHub will prompt you to create a PR for that branch.

Got it, I see thanks!

sidcha commented 1 year ago

PR #90 merged!