@rfuchs commented on this pull request.
An example of the intended usage would be helpful
> + } else if(str_eq(&key, "all")) {
+ ng_flags->all_legs = 1;
+ } else
I think this is redundant? Just putting the word "all" in the argument string
would already lead to "all" being put in the flags list without the need for
special handling, no?
> + else if(str_eq(&key, "from-tags")) {
+ err = "missing value";
+ if(!val.s)
+ goto error;
+
+ if(!ng_flags->from_tags) {
+ ng_flags->from_tags =
+
bencode_list(ng_flags->dict->buffer);
+ }
+
bencode_list_add_str(ng_flags->from_tags, &val);
+ } else
Ok if this is how you want to do it, but some suggestions for alternatives:
`subscribe request` can take either a single `from-tag` or multiple `from-tags`
as a list, and would treat them the same. So here you could also treat
"from-tag" and "from-tags" the same, and making building the list conditional
on the opmode.
Or: omit handling altogether and require a modern version of rtpengine which
parses the flags internally (`parsed_by_module` false). In this case you can
use syntax like `rtpengine_blah("flag flag ... from-tags=[tag1 tag2 tag3] ...")`
Up to you, just a hint.
> + {"rtpengine_subscribe_offer",
> (cmd_function)rtpengine_subscribe_request_wrap_f, 4,
+ fixup_rtpengine_subscribe_request_v,
fixup_free_rtpengine_subscribe_request_v, ANY_ROUTE},
+ {"rtpengine_subscribe_offer",
(cmd_function)rtpengine_subscribe_request_wrap_f, 5,
+ fixup_rtpengine_subscribe_request_v,
fixup_free_rtpengine_subscribe_request_v, ANY_ROUTE},
+ {"rtpengine_subscribe_answer",
(cmd_function)rtpengine_subscribe_answer_wrap_f, 1,
+ fixup_spve_null, fixup_free_spve_null, ANY_ROUTE},
+ {"rtpengine_subscribe_answer",
(cmd_function)rtpengine_subscribe_answer_wrap_f, 2,
+ fixup_spve_spve, fixup_free_spve_spve, ANY_ROUTE},
+ {"rtpengine_unsubscribe", (cmd_function)rtpengine_unsubscribe_wrap_f, 1,
+ fixup_spve_null, fixup_free_spve_null, ANY_ROUTE},
+ {"rtpengine_unsubscribe", (cmd_function)rtpengine_unsubscribe_wrap_f, 2,
+ fixup_spve_spve, fixup_free_spve_spve, ANY_ROUTE},
These would have to be added to the documentation
--
Reply to this email directly or view it on GitHub:
https://github.com/kamailio/kamailio/pull/4274#pullrequestreview-2904627533
You are receiving this because you are subscribed to this thread.
Message ID: <kamailio/kamailio/pull/4274/review/2904627...@github.com>_______________________________________________
Kamailio - Development Mailing List -- sr-dev@lists.kamailio.org
To unsubscribe send an email to sr-dev-le...@lists.kamailio.org
Important: keep the mailing list in the recipients, do not reply only to the
sender!
