From f6615cd0b0d441d01b6fde02b8960f3c92d260e4 Mon Sep 17 00:00:00 2001 From: "Jonathan Thorpe (Sony)" Date: Mon, 10 Feb 2025 16:57:42 +0000 Subject: [PATCH 1/3] Fix remove_sequence_item control protocol method --- Development/cmake/NmosCppTest.cmake | 1 + Development/nmos/control_protocol_methods.cpp | 13 +- Development/nmos/control_protocol_methods.h | 2 +- Development/nmos/control_protocol_state.cpp | 8 +- .../test/control_protocol_methods_test.cpp | 146 ++++++++++++++++++ 5 files changed, 164 insertions(+), 6 deletions(-) create mode 100644 Development/nmos/test/control_protocol_methods_test.cpp diff --git a/Development/cmake/NmosCppTest.cmake b/Development/cmake/NmosCppTest.cmake index 31fa6c21..e75582eb 100644 --- a/Development/cmake/NmosCppTest.cmake +++ b/Development/cmake/NmosCppTest.cmake @@ -44,6 +44,7 @@ set(NMOS_CPP_TEST_NMOS_TEST_SOURCES nmos/test/capabilities_test.cpp nmos/test/channels_test.cpp nmos/test/control_protocol_test.cpp + nmos/test/control_protocol_methods_test.cpp nmos/test/did_sdid_test.cpp nmos/test/event_type_test.cpp nmos/test/json_validator_test.cpp diff --git a/Development/nmos/control_protocol_methods.cpp b/Development/nmos/control_protocol_methods.cpp index 9894cd5e..153d953b 100644 --- a/Development/nmos/control_protocol_methods.cpp +++ b/Development/nmos/control_protocol_methods.cpp @@ -296,7 +296,7 @@ namespace nmos } // Delete sequence item - web::json::value remove_sequence_item(nmos::resources& resources, const nmos::resource& resource, const web::json::value& arguments, bool is_deprecated, get_control_protocol_class_descriptor_handler get_control_protocol_class_descriptor, slog::base_gate& gate) + web::json::value remove_sequence_item(nmos::resources& resources, const nmos::resource& resource, const web::json::value& arguments, bool is_deprecated, get_control_protocol_class_descriptor_handler get_control_protocol_class_descriptor, control_protocol_property_changed_handler property_changed, slog::base_gate& gate) { // note, model mutex is already locked by the outer function, so access to control_protocol_resources is OK... @@ -309,6 +309,11 @@ namespace nmos const auto& property = find_property_descriptor(details::parse_nc_property_id(property_id), details::parse_nc_class_id(nmos::fields::nc::class_id(resource.data)), get_control_protocol_class_descriptor); if (!property.is_null()) { + if (nmos::fields::nc::is_read_only(property)) + { + return details::make_nc_method_result({ nc_method_status::read_only }); + } + const auto& data = resource.data.at(nmos::fields::nc::name(property)); if (!nmos::fields::nc::is_sequence(property) || data.is_null() || !data.is_array()) @@ -327,6 +332,12 @@ namespace nmos auto& sequence = resource.data[nmos::fields::nc::name(property)].as_array(); sequence.erase(index); + // do notification that the specified property has changed + if (property_changed) + { + property_changed(resource, nmos::fields::nc::name(property), index); + } + }, make_property_changed_event(nmos::fields::nc::oid(resource.data), { { details::parse_nc_property_id(property_id), nc_property_change_type::type::sequence_item_removed, nc_id(index) } })); return details::make_nc_method_result({ is_deprecated ? nmos::nc_method_status::method_deprecated : nmos::fields::nc::is_deprecated(property) ? nc_method_status::property_deprecated : nc_method_status::ok }); diff --git a/Development/nmos/control_protocol_methods.h b/Development/nmos/control_protocol_methods.h index 957f9362..69f7b7c4 100644 --- a/Development/nmos/control_protocol_methods.h +++ b/Development/nmos/control_protocol_methods.h @@ -23,7 +23,7 @@ namespace nmos // Add item to sequence web::json::value add_sequence_item(nmos::resources& resources, const nmos::resource& resource, const web::json::value& arguments, bool is_deprecated, get_control_protocol_class_descriptor_handler get_control_protocol_class_descriptor, get_control_protocol_datatype_descriptor_handler, control_protocol_property_changed_handler property_changed, slog::base_gate& gate); // Delete sequence item - web::json::value remove_sequence_item(nmos::resources& resources, const nmos::resource& resource, const web::json::value& arguments, bool is_deprecated, get_control_protocol_class_descriptor_handler get_control_protocol_class_descriptor, slog::base_gate& gate); + web::json::value remove_sequence_item(nmos::resources& resources, const nmos::resource& resource, const web::json::value& arguments, bool is_deprecated, get_control_protocol_class_descriptor_handler get_control_protocol_class_descriptor, control_protocol_property_changed_handler property_changed, slog::base_gate& gate); // Get sequence length web::json::value get_sequence_length(nmos::resources& resources, const nmos::resource& resource, const web::json::value& arguments, bool is_deprecated, get_control_protocol_class_descriptor_handler get_control_protocol_class_descriptor, slog::base_gate& gate); diff --git a/Development/nmos/control_protocol_state.cpp b/Development/nmos/control_protocol_state.cpp index dd4b1911..11f91662 100644 --- a/Development/nmos/control_protocol_state.cpp +++ b/Development/nmos/control_protocol_state.cpp @@ -122,11 +122,11 @@ namespace nmos return add_sequence_item(resources, resource, arguments, is_deprecated, get_control_protocol_class_descriptor, get_control_protocol_datatype_descriptor, property_changed, gate); }; } - nmos::experimental::control_protocol_method_handler make_nc_remove_sequence_item_handler(get_control_protocol_class_descriptor_handler get_control_protocol_class_descriptor) + nmos::experimental::control_protocol_method_handler make_nc_remove_sequence_item_handler(get_control_protocol_class_descriptor_handler get_control_protocol_class_descriptor, control_protocol_property_changed_handler property_changed) { - return [get_control_protocol_class_descriptor](nmos::resources& resources, const nmos::resource& resource, const web::json::value& arguments, bool is_deprecated, slog::base_gate& gate) + return [get_control_protocol_class_descriptor, property_changed](nmos::resources& resources, const nmos::resource& resource, const web::json::value& arguments, bool is_deprecated, slog::base_gate& gate) { - return remove_sequence_item(resources, resource, arguments, is_deprecated, get_control_protocol_class_descriptor, gate); + return remove_sequence_item(resources, resource, arguments, is_deprecated, get_control_protocol_class_descriptor, property_changed, gate); }; } nmos::experimental::control_protocol_method_handler make_nc_get_sequence_length_handler(get_control_protocol_class_descriptor_handler get_control_protocol_class_descriptor) @@ -230,7 +230,7 @@ namespace nmos { nc_object_get_sequence_item_method_id, details::make_nc_get_sequence_item_handler(get_control_protocol_class_descriptor) }, { nc_object_set_sequence_item_method_id, details::make_nc_set_sequence_item_handler(get_control_protocol_class_descriptor, get_control_protocol_datatype_descriptor, property_changed) }, { nc_object_add_sequence_item_method_id, details::make_nc_add_sequence_item_handler(get_control_protocol_class_descriptor, get_control_protocol_datatype_descriptor, property_changed) }, - { nc_object_remove_sequence_item_method_id, details::make_nc_remove_sequence_item_handler(get_control_protocol_class_descriptor) }, + { nc_object_remove_sequence_item_method_id, details::make_nc_remove_sequence_item_handler(get_control_protocol_class_descriptor, property_changed) }, { nc_object_get_sequence_length_method_id, details::make_nc_get_sequence_length_handler(get_control_protocol_class_descriptor) } }), // NcObject events diff --git a/Development/nmos/test/control_protocol_methods_test.cpp b/Development/nmos/test/control_protocol_methods_test.cpp new file mode 100644 index 00000000..407d3937 --- /dev/null +++ b/Development/nmos/test/control_protocol_methods_test.cpp @@ -0,0 +1,146 @@ +// The first "test" is of course whether the header compiles standalone +#include "boost/iostreams/stream.hpp" +#include "boost/iostreams/device/null.hpp" +#include "nmos/control_protocol_resource.h" +#include "nmos/control_protocol_resources.h" +#include "nmos/control_protocol_methods.h" +#include "nmos/control_protocol_state.h" +#include "nmos/control_protocol_typedefs.h" +#include "nmos/control_protocol_utils.h" +#include "nmos/is12_versions.h" +#include "nmos/json_fields.h" +#include "nmos/log_gate.h" +#include "nmos/slog.h" +#include "bst/test/test.h" + + +//////////////////////////////////////////////////////////////////////////////////////////// +BST_TEST_CASE(testRemoveSequenceItem) +{ + using web::json::value_of; + using web::json::value; + + bool property_changed_called = false; + + boost::iostreams::stream< boost::iostreams::null_sink > null_ostream((boost::iostreams::null_sink())); + + nmos::experimental::log_model log_model; + nmos::experimental::log_gate gate(null_ostream, null_ostream, log_model); + + nmos::resources resources; + nmos::control_protocol_property_changed_handler property_changed = [&property_changed_called](const nmos::resource& resource, const utility::string_t& property_name, int index) + { + // check that the property changed handler gets called + property_changed_called = true; + }; + nmos::experimental::control_protocol_state control_protocol_state(property_changed); + nmos::get_control_protocol_class_descriptor_handler get_control_protocol_class_descriptor = nmos::make_get_control_protocol_class_descriptor_handler(control_protocol_state); + + + // Create simple non-standard class with writable sequence property + const auto writable_sequence_class_id = nmos::make_nc_class_id(nmos::nc_worker_class_id, -1234, { 1000 }); + const web::json::field_as_array writable_value{ U("writableValue") }; + { + // Gain control class property descriptors + std::vector writable_sequence_property_descriptors = { nmos::experimental::make_control_class_property_descriptor(U("Writable sequence"), { 3, 1 }, writable_value, U("NcInt16"), false, false, true, false, web::json::value::null()) }; + + // create Gain control class descriptor + auto writable_sequence_class_descriptor = nmos::experimental::make_control_class_descriptor(U("Writable sequence class descriptor"), writable_sequence_class_id, U("WritableSequence"), writable_sequence_property_descriptors); + + // insert Gain control class descriptor to global state, which will be used by the control_protocol_ws_message_handler to process incoming ws message + control_protocol_state.insert(writable_sequence_class_descriptor); + } + // helper function to create Gain control instance + auto make_writable_sequence = [&writable_value, &writable_sequence_class_id](nmos::nc_oid oid, nmos::nc_oid owner, const utility::string_t& role, const utility::string_t& user_label, const utility::string_t& description) + { + auto data = nmos::details::make_nc_worker(writable_sequence_class_id, oid, true, owner, role, value::string(user_label), description, web::json::value::null(), web::json::value::null(), true); + auto values = value::array(); + web::json::push_back(values, value::number(10)); + web::json::push_back(values, value::number(9)); + web::json::push_back(values, value::number(8)); + data[writable_value] = values; + + return nmos::control_protocol_resource{ nmos::is12_versions::v1_0, nmos::types::nc_worker, std::move(data), true }; + }; + + // Create Device Model + // root + auto root_block = nmos::make_root_block(); + auto oid = nmos::root_block_oid; + // root, ClassManager + auto class_manager = nmos::make_class_manager(++oid, control_protocol_state); + auto receiver_block_oid = ++oid; + // root, receivers + auto receivers = nmos::make_block(receiver_block_oid, nmos::root_block_oid, U("receivers"), U("Receivers"), U("Receivers block")); + auto receivers_id = receivers.id; + + // root, receivers, mon1 + auto monitor1 = nmos::make_receiver_monitor(++oid, true, receiver_block_oid, U("mon1"), U("monitor 1"), U("monitor 1"), value::null()); + // root, receivers, mon2 + auto monitor2 = nmos::make_receiver_monitor(++oid, true, receiver_block_oid, U("mon2"), U("monitor 2"), U("monitor 2"), value::null()); + + auto writable_sequence = make_writable_sequence(++oid, nmos::root_block_oid, U("writableSequence"), U("writable sequence"), U("writable sequence")); + auto writable_sequence_id = writable_sequence.id; + + nmos::push_back(receivers, monitor1); + // add example-control to root-block + nmos::push_back(receivers, monitor2); + // add stereo-gain to root-block + nmos::push_back(root_block, receivers); + // add class-manager to root-block + nmos::push_back(root_block, class_manager); + // add writable sequence to root block + nmos::push_back(root_block, writable_sequence); + insert_resource(resources, std::move(root_block)); + insert_resource(resources, std::move(class_manager)); + insert_resource(resources, std::move(receivers)); + insert_resource(resources, std::move(monitor1)); + insert_resource(resources, std::move(monitor2)); + insert_resource(resources, std::move(writable_sequence)); + + // Attempt to remove a member from a block - read only error expected + { + property_changed_called = false; + + auto block_members_property_id = value_of({ + { U("level"), nmos::nc_block_members_property_id.level }, + { U("index"), nmos::nc_block_members_property_id.index}, + }); + + auto arguments = value_of({ + { nmos::fields::nc::id, block_members_property_id }, + { nmos::fields::nc::index, 0} + }); + + auto resource = nmos::find_resource(resources, receivers_id); + BST_CHECK_NE(resources.end(), resource); + auto result = nmos::remove_sequence_item(resources, *resource, arguments, false, get_control_protocol_class_descriptor, property_changed, gate); + + // Expect read only error, and for property changed not to be called + BST_CHECK_EQUAL(false, property_changed_called); + BST_CHECK_EQUAL(nmos::nc_method_status::read_only, nmos::fields::nc::status(result)); + } + + // Remove writable sequence item - succss and property_changed event expected + { + property_changed_called = false; + + auto writable_sequence_property_id = value_of({ + { U("level"), 3 }, + { U("index"), 1}, + }); + + auto arguments = value_of({ + { nmos::fields::nc::id, writable_sequence_property_id }, + { nmos::fields::nc::index, 1} + }); + + auto resource = nmos::find_resource(resources, writable_sequence_id); + BST_CHECK_NE(resources.end(), resource); + auto result = nmos::remove_sequence_item(resources, *resource, arguments, false, get_control_protocol_class_descriptor, property_changed, gate); + + // Expect success, and property changed event + BST_CHECK_EQUAL(true, property_changed_called); + BST_CHECK_EQUAL(nmos::nc_method_status::ok, nmos::fields::nc::status(result)); + } +} From 90b956fcfb11ddbde9e9a3fba13cc4e84f7cfc9b Mon Sep 17 00:00:00 2001 From: "Jonathan Thorpe (Sony)" Date: Mon, 10 Feb 2025 17:00:50 +0000 Subject: [PATCH 2/3] Formatting --- Development/cmake/NmosCppTest.cmake | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Development/cmake/NmosCppTest.cmake b/Development/cmake/NmosCppTest.cmake index e75582eb..45d953d3 100644 --- a/Development/cmake/NmosCppTest.cmake +++ b/Development/cmake/NmosCppTest.cmake @@ -44,7 +44,7 @@ set(NMOS_CPP_TEST_NMOS_TEST_SOURCES nmos/test/capabilities_test.cpp nmos/test/channels_test.cpp nmos/test/control_protocol_test.cpp - nmos/test/control_protocol_methods_test.cpp + nmos/test/control_protocol_methods_test.cpp nmos/test/did_sdid_test.cpp nmos/test/event_type_test.cpp nmos/test/json_validator_test.cpp From b99e9d6bf26ce6d8aabf8972a0f385c7cd1f0214 Mon Sep 17 00:00:00 2001 From: "Jonathan Thorpe (Sony)" Date: Mon, 10 Feb 2025 17:03:20 +0000 Subject: [PATCH 3/3] Update comments --- .../nmos/test/control_protocol_methods_test.cpp | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/Development/nmos/test/control_protocol_methods_test.cpp b/Development/nmos/test/control_protocol_methods_test.cpp index 407d3937..d49370ee 100644 --- a/Development/nmos/test/control_protocol_methods_test.cpp +++ b/Development/nmos/test/control_protocol_methods_test.cpp @@ -41,16 +41,16 @@ BST_TEST_CASE(testRemoveSequenceItem) const auto writable_sequence_class_id = nmos::make_nc_class_id(nmos::nc_worker_class_id, -1234, { 1000 }); const web::json::field_as_array writable_value{ U("writableValue") }; { - // Gain control class property descriptors + // Writable sequence_class property descriptors std::vector writable_sequence_property_descriptors = { nmos::experimental::make_control_class_property_descriptor(U("Writable sequence"), { 3, 1 }, writable_value, U("NcInt16"), false, false, true, false, web::json::value::null()) }; - // create Gain control class descriptor + // create writable_sequence class descriptor auto writable_sequence_class_descriptor = nmos::experimental::make_control_class_descriptor(U("Writable sequence class descriptor"), writable_sequence_class_id, U("WritableSequence"), writable_sequence_property_descriptors); - // insert Gain control class descriptor to global state, which will be used by the control_protocol_ws_message_handler to process incoming ws message + // insert writable_sequence class descriptor to global state, which will be used by the control_protocol_ws_message_handler to process incoming ws message control_protocol_state.insert(writable_sequence_class_descriptor); } - // helper function to create Gain control instance + // helper function to create writable_sequence object auto make_writable_sequence = [&writable_value, &writable_sequence_class_id](nmos::nc_oid oid, nmos::nc_oid owner, const utility::string_t& role, const utility::string_t& user_label, const utility::string_t& description) { auto data = nmos::details::make_nc_worker(writable_sequence_class_id, oid, true, owner, role, value::string(user_label), description, web::json::value::null(), web::json::value::null(), true); @@ -121,7 +121,7 @@ BST_TEST_CASE(testRemoveSequenceItem) BST_CHECK_EQUAL(nmos::nc_method_status::read_only, nmos::fields::nc::status(result)); } - // Remove writable sequence item - succss and property_changed event expected + // Remove writable sequence item - success and property_changed event expected { property_changed_called = false;