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

Adding grammar for precondition a attributes #100

Open
wants to merge 8 commits into
base: master
Choose a base branch
from

Conversation

hyavari
Copy link

@hyavari hyavari commented Sep 12, 2023

Hi, I've just added the required grammar for parsing precondition a attributes based on RFC 3312.

Sample:

v=0
.....
a=curr:qos local none
a=curr:qos remote none
a=des:qos mandatory local sendrecv
a=des:qos none remote sendrecv
a=sendrecv
.....

Comment on lines +515 to +525
// RFC 3312 precondition
push: 'precondition',
reg: /^(curr|des|conf):(qos|\S*)(?: (mandatory|optional|none|failure|unknown))? (e2e|local|remote) (none|sendrecv|send|recv)/,
names: ['state', 'type', 'strength', 'status', 'direction'],
format: function (o) {
var str = '%s:%s ';
str += o.strength != null ? '%s ' : '%v';
str += o.status != null ? '%s ' : '';
str += o.direction != null ? '%s ' : '';
return str;
},
Copy link
Owner

Choose a reason for hiding this comment

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

This seems to be the part of the thing that you are actually adding. From a quick scan, looks sensible.
Are you able to add some tests for it?

Copy link
Author

Choose a reason for hiding this comment

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

Hi, I tried but not sure why tests are not runnable. I'll spend more time.

lib/grammar.js Outdated Show resolved Hide resolved
@hyavari
Copy link
Author

hyavari commented Sep 12, 2023

I added parse and write tests for precondition added grammar.

@clux
Copy link
Owner

clux commented Sep 12, 2023

Thanks for this. This looks great to me.

Now for the awkward part. It's actually been a while since I've had to do a release on this, and travis has shutdown since :|
Will need to spend some time to fixup CI now.

@hyavari
Copy link
Author

hyavari commented Sep 12, 2023

Hi Eirik,
I've just added the MRCP attributes support as well (grammar + test).

@clux
Copy link
Owner

clux commented Sep 16, 2023

Hey, i've updated testing setup now, if you're able to update (nm, i can do it from here) - and it's all green - i can try to push this out later :-)

@codecov-commenter

This comment was marked as spam.

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.

None yet

3 participants