Skip to content

IS-05: Testing scenario when two receivers get bulk-staged for relati…#73

Open
bakaleks wants to merge 1 commit intoAMWA-TV:masterfrom
bakaleks:bulk_activation_sony_bug
Open

IS-05: Testing scenario when two receivers get bulk-staged for relati…#73
bakaleks wants to merge 1 commit intoAMWA-TV:masterfrom
bakaleks:bulk_activation_sony_bug

Conversation

@bakaleks
Copy link
Copy Markdown

@bakaleks bakaleks commented Jan 9, 2019

…ve scheduled activation. Both of them should be activated. Testing with Sony's nmos-cpp-node sample application with two registered receivers

…ve scheduled activation. Both of them should be activated. Testing with Sony's nmos-cpp-node sample application with two registered receivers
@bakaleks
Copy link
Copy Markdown
Author

bakaleks commented Jan 9, 2019

Testing with following change sony/nmos-cpp#43

Copy link
Copy Markdown
Contributor

@garethsb garethsb left a comment

Choose a reason for hiding this comment

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

Thanks, @bakaleks. I think this will be a great addition to the test suite - it demonstrated a bug in my implementation as you said.

I'd like to see a bit of refactoring to eliminate the duplication with IS05Utils.check_perform_relative_activation if possible. What do you think, @andrewbonney?

@andrewbonney
Copy link
Copy Markdown
Contributor

I'll ask @simonrankine to take a quick scan. Otherwise as this is your first contribution @bakaleks we'll probably have to ask you to fill in an IPR form as-per point 3 in https://github.com/AMWA-TV/nmos-testing/blob/master/CONTRIBUTING.pdf. It might be easiest for us to discuss in person next week.

@simonrankine
Copy link
Copy Markdown
Contributor

Over-all this looks like a good and useful thing to have in the test suite, thanks very much for your contribution. What I would say is that there seems to be a lot with duplication with check_bulk_stage in the same file, to the point where I think I'd like it to be combined with check_bulk_active_relative before we merge, or this code is going to get mighty hard to maintain.

@andrewbonney andrewbonney added the pending refactor PR has been reviewed and requires some modifications before merging label Feb 7, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

pending refactor PR has been reviewed and requires some modifications before merging

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants