Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

res_pjsip_pubsub: Add new pubsub module capabilities. #82

Merged
merged 1 commit into from May 18, 2023

Conversation

InterLinked1
Copy link
Contributor

The existing res_pjsip_pubsub APIs are somewhat limited in what they can do. This adds a few API extensions that make it possible for PJSIP pubsub modules to implement richer features than is currently possible.

  • Allow pubsub modules to get a handle to pjsip_rx_data on subscription
  • Allow pubsub modules to run a callback when a subscription is renewed
  • Allow pubsub modules to run a callback for outgoing NOTIFYs, with a handle to the tdata, so that modules can append their own headers to the NOTIFYs

This change does not add any features directly, but makes possible several new features that will be added in future changes.

Resolves: #81
ASTERISK-30485 #close
Imported from Gerrit: https://gerrit.asterisk.org/c/asterisk/+/20031

Master-Only: True

include/asterisk/res_pjsip_pubsub.h Outdated Show resolved Hide resolved
include/asterisk/res_pjsip_pubsub.h Outdated Show resolved Hide resolved
res/res_pjsip_pubsub.c Outdated Show resolved Hide resolved
res/res_pjsip_pubsub.c Outdated Show resolved Hide resolved
res/res_pjsip_pubsub.c Outdated Show resolved Hide resolved
res/res_pjsip_pubsub.c Show resolved Hide resolved
jcolp
jcolp previously approved these changes May 18, 2023
@InterLinked1
Copy link
Contributor Author

@jcolp Previously the gate tests were failing but this last one, it seems the unit tests also failed. It says Building failed. See /github/workspace/cache/output//build.err for more details. but I can't find where that would be. Is this an actual failure to be concerned about here or just a kink in the CI process still being worked out?

@jcolp
Copy link
Member

jcolp commented May 18, 2023

It's a legit build failure:

[CC] res_pjsip_sdp_rtp.c -> res_pjsip_sdp_rtp.o res_pjsip_pubsub.c: In function ‘pubsub_on_rx_refresh’: res_pjsip_pubsub.c:4102:17: warning: assignment discards ‘const’ qualifier from pointer target type [-Wdiscarded-qualifiers] 4102 | handler = sub_tree->root->handler; | ^ res_pjsip_pubsub.c:4125:52: error: ‘endpoint’ undeclared (first use in this function); did you mean ‘endrpcent’? 4125 | resp = build_resource_tree(endpoint, handler, resource, &tree, | ^~~~~~~~ | endrpcent res_pjsip_pubsub.c:4125:52: note: each undeclared identifier is reported only once for each function it appears in make[1]: *** [/home/jcolp/development/asterisk/github/Makefile.rules:165: res_pjsip_pubsub.o] Error 1 make[1]: *** Waiting for unfinished jobs.... make: *** [Makefile:396: res] Error 2

@InterLinked1
Copy link
Contributor Author

It's a legit build failure:

Thanks, how did you get to that file in the logs? Is that only accessible to repo members?

@jcolp
Copy link
Member

jcolp commented May 18, 2023

I didn't, I built it locally. I've asked George to comment on the best way to access that.

@gtjoseph
Copy link
Member

I didn't, I built it locally. I've asked George to comment on the best way to access that.

It's not as easy as it used to be and it's on my list to fix but in the mean time...

  • Click on the details for the failing check.
  • Click on the job summary at the top left of the page
  • Scroll down to Artifacts and click on Unit Test Output.

It's a zip file that will usually cause the browser to prompt to open or download. If you click open, many distros will automatically open an archive browser that'll let you read the files inside without having to manually unzip the file first.

I'm going to go back to keeping that output in the initial details.

@InterLinked1
Copy link
Contributor Author

I didn't, I built it locally. I've asked George to comment on the best way to access that.

It's not as easy as it used to be and it's on my list to fix but in the mean time...

  • Click on the details for the failing check.
  • Click on the job summary at the top left of the page
  • Scroll down to Artifacts and click on Unit Test Output.

It's a zip file that will usually cause the browser to prompt to open or download. If you click open, many distros will automatically open an archive browser that'll let you read the files inside without having to manually unzip the file first.

I'm going to go back to keeping that output in the initial details.

Thanks George, that worked for me. If this is documented on the commit workflow page on the wiki, I think it's fine, I was just struggling to find the build output like we had access to on Jenkins, and on GitHub was used to seeing the build output on the main page. If people test their changes locally first this shouldn't come up too often anyways.

The existing res_pjsip_pubsub APIs are somewhat limited in
what they can do. This adds a few API extensions that make
it possible for PJSIP pubsub modules to implement richer
features than is currently possible.

* Allow pubsub modules to get a handle to pjsip_rx_data on subscription
* Allow pubsub modules to run a callback when a subscription is renewed
* Allow pubsub modules to run a callback for outgoing NOTIFYs, with
  a handle to the tdata, so that modules can append their own headers
  to the NOTIFYs

This change does not add any features directly, but makes possible
several new features that will be added in future changes.

Resolves: asterisk#81
ASTERISK-30485 #close

Master-Only: True
@InterLinked1
Copy link
Contributor Author

No cherry-picks required

@gtjoseph gtjoseph merged commit 659f2aa into asterisk:master May 18, 2023
17 checks passed
@MikeNaso MikeNaso mentioned this pull request Aug 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[improvement]: Enhance and add additional PJSIP pubsub callbacks
3 participants