Skip to content

Added functions to register callbacks to the param_sniffer()#49

Draft
kivkiv12345 wants to merge 1 commit intomasterfrom
param_sniffer_callbacks
Draft

Added functions to register callbacks to the param_sniffer()#49
kivkiv12345 wants to merge 1 commit intomasterfrom
param_sniffer_callbacks

Conversation

@kivkiv12345
Copy link
Contributor

@kivkiv12345 kivkiv12345 commented Nov 4, 2025

This is your chance to stop me @troelsjessen! :)

There's still a couple of things I want to change.
But if you have any major grievances with the general concept, you can let me know before I continue.

I've tested it ad-hoc, with the following function/callbacks:

void my_sniffer_callback(param_t * param, void *context) {
    printf("AAA %s %s\n", param->name, (char*)context);
}

void my_other_sniffer_callback(param_t * param, void *context) {
    printf("BBB %s%s\n", param->name, (char*)context);
}

void register_callbacks(void) {
    param_sniffer_register_callback(my_sniffer_callback, "hello there");
    param_sniffer_register_callback(my_other_sniffer_callback, ". What is up?");
}

void unregister_callbacks(void) {
    param_sniffer_unregister_callback(my_sniffer_callback);
    param_sniffer_unregister_callback(my_other_sniffer_callback);
}

pull after calling register_callbacks():

BBB csp_buf_out. What is up?
AAA csp_buf_out hello there
BBB csp_conn_out. What is up?
AAA csp_conn_out hello there
BBB csp_conn_ovf. What is up?
AAA csp_conn_ovf hello there
BBB csp_conn_noroute. What is up?
  51:124  csp_buf_out          = AAA csp_conn_noroute hello there
0                   
  52:124  csp_conn_out         = 0                   
BBB csp_inval_reply. What is up?
...
AAA csp_print_rdp hello there
BBB csp_print_packet. What is up?
AAA csp_print_packet hello there

Pull after calling unregister_callbacks():

  51:124  csp_buf_out          = 0                   
  52:124  csp_conn_out         = 0                   
...
  58:124  csp_print_rdp        = 0                   
  59:124  csp_print_packet     = 0                   
 1001:124  test_array_param     = [0 1 2 3 4 5 6 7]   
 1002:124  test_str             = 

Valgrind tested, no errors.

int param_sniffer_log(void * ctx, param_queue_t *queue, param_t *param, int offset, void *reader, csp_timestamp_t *timestamp);
void param_sniffer_init(int add_logfile);

/* TODO Kevin: We should probably pass along `*queue`, `offset` and `*timestamp` as well. */
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we "probably should", why not do it ? :)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I take it you agree then? I will do it!


int param_sniffer_register_callback(param_sniffer_callback_f callback, void * context) {

/* TODO Kevin: What about registering the same callback multiple times? What if it has different contexts? */
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about that ? This is a way for users of this API to register things, we can't be responsible for what they actually register, can we ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I left that comment wondering how I should handle it in param_sniffer_UNregister_callback(). Do we unregister all callbacks to that function, or only the first match?
Do we error if that function is already registered?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A solution would be to update the
int param_sniffer_unregister_callback(param_sniffer_callback_f callback)
to:
int param_sniffer_unregister_callback(param_sniffer_callback_f callback, void * context)
and then test for both the callbackand contextto find the correct element to unregister...

@kivkiv12345
Copy link
Contributor Author

@jeanbaptistelab Maybe it's overkill to hide SLIST? Especially since I've been considering exposing the callback collection itself as well. But I think that can be a future addition :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants