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.

Sign up for GitHub

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

Jump to bottom

IS-14 Refactor bulkProperties methods #8

Merged
merged 8 commits into from
Jul 14, 2024
Merged

IS-14 Refactor bulkProperties methods #8

merged 8 commits into from
Jul 14, 2024

Conversation

jonathan-r-thorpe
Copy link
Collaborator

bulkProperties methods now look up the method descriptor in the BulkPropertiesManager, rather then invoking them directly.

lo-simon
lo-simon requested changes Jul 1, 2024
View reviewed changes
Copy link
Owner

@lo-simon lo-simon left a comment

Choose a reason for hiding this comment

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

LGYM, just a few blanks to remove

Development/nmos/configuration_api.cpp Outdated Show resolved Hide resolved
Development/nmos/configuration_api.cpp Outdated Show resolved Hide resolved
Development/nmos/configuration_api.cpp Outdated Show resolved Hide resolved
jonathan-r-thorpe and others added 3 commits July 2, 2024 11:46
@jonathan-r-thorpe @lo-simon
Apply suggestions from code review
9d25614 
Co-authored-by: Simon Lo <simon.lo@sony.com>
@jonathan-r-thorpe
Updated according to latest specification
6b92890 
(cherry picked from commit c38fb2a)
@jonathan-r-thorpe
Add control_protocol_state to device configuration handlers
fced9d8 
(cherry picked from commit 6de883bac8a5f8ab4d09f3b36ffb8e352771e270)
lo-simon
lo-simon requested changes Jul 3, 2024
View reviewed changes
Copy link
Owner

@lo-simon lo-simon left a comment

Choose a reason for hiding this comment

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

LGTM, if we can provide examples of the bulk handlers, we may be able to hide the control_protocol_state with callback for those handlers.

Development/nmos/control_protocol_typedefs.h Outdated Show resolved Hide resolved
lo-simon
lo-simon approved these changes Jul 14, 2024
View reviewed changes
Copy link
Owner

@lo-simon lo-simon left a comment

Choose a reason for hiding this comment

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

LGTM

@lo-simon lo-simon merged commit 226dc59 into is-14 Jul 14, 2024
18 of 22 checks passed
@lo-simon lo-simon deleted the is-14-refactor-method-invokation branch July 14, 2024 11:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lo-simon lo-simon lo-simon approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@jonathan-r-thorpe @lo-simon