From c8dd5926096514685abd0c9ae8e60c5bcf096776 Mon Sep 17 00:00:00 2001 From: "Rule Timothy (CC/EMT2)" Date: Thu, 22 Feb 2024 18:00:13 +0100 Subject: [PATCH] Prevent echo of binary data when reset() is not called on a signal. Signed-off-by: Rule Timothy (VM/EMT3) --- dse/mocks/simmock.c | 6 ++ dse/modelc/controller/controller.c | 12 ++++ dse/modelc/controller/controller.h | 1 + dse/modelc/model.h | 7 +- dse/modelc/model/model.c | 4 ++ dse/modelc/model/ncodec.c | 1 + dse/modelc/model/signal.c | 3 + tests/cmocka/CMakeLists.txt | 1 + .../model/interface/binary_stream_reset.yaml | 63 +++++++++++++++++ tests/cmocka/model/interface/test_model_api.c | 68 +++++++++++++++++++ tests/cmocka/model/test_signal.c | 4 ++ 11 files changed, 167 insertions(+), 3 deletions(-) create mode 100644 tests/cmocka/model/interface/binary_stream_reset.yaml diff --git a/dse/mocks/simmock.c b/dse/mocks/simmock.c index b1f70d7..500139e 100644 --- a/dse/mocks/simmock.c +++ b/dse/mocks/simmock.c @@ -95,6 +95,7 @@ static void __free_stub_sv(SignalVector* sv) if (sv->binary) free(sv->binary); if (sv->length) free(sv->length); if (sv->buffer_size) free(sv->buffer_size); + if (sv->reset_called) free(sv->reset_called); if (sv->mime_type) free(sv->mime_type); if (sv->ncodec) free(sv->ncodec); @@ -266,6 +267,7 @@ static SignalVector* __stub_sv(SignalVector* sv) stub_sv->binary = calloc(stub_sv->count, sizeof(void*)); stub_sv->length = calloc(stub_sv->count, sizeof(uint32_t)); stub_sv->buffer_size = calloc(stub_sv->count, sizeof(uint32_t)); + stub_sv->reset_called = calloc(stub_sv->count, sizeof(bool)); stub_sv->mime_type = calloc(stub_sv->count, sizeof(const char*)); stub_sv->ncodec = calloc(stub_sv->count, sizeof(NCODEC*)); @@ -403,11 +405,15 @@ int simmock_step(SimMock* mock, bool assert_rc) model->sv_network->append(model->sv_network, i, mock->sv_network_rx->binary[i], mock->sv_network_rx->length[i]); + model->sv_network->reset_called[i] = false; } } rc |= modelc_step(model->mi, mock->step_size); if (mock->sv_network_rx && mock->sv_network_tx) { for (uint32_t i = 0; i < model->sv_network->count; i++) { + if (model->sv_network->reset_called[i] == false) { + model->sv_network->length[i] = 0; + } mock->sv_network_tx->append(mock->sv_network_tx, i, model->sv_network->binary[i], model->sv_network->length[i]); } diff --git a/dse/modelc/controller/controller.c b/dse/modelc/controller/controller.c index 37313a7..81b1618 100644 --- a/dse/modelc/controller/controller.c +++ b/dse/modelc/controller/controller.c @@ -118,6 +118,9 @@ static int __marshal__adapter2model(void* _mfc, void* _spec) sm[si].signal->bin_size); /* Indicate the binary object was consumed. */ sm[si].signal->bin_size = 0; + /* Set the trigger to detect if the binary object is correctly + operated by the Model (i.e. calls reset()).*/ + mfc->signal_value_binary_reset_called[si] = false; } } free(sm); @@ -140,6 +143,15 @@ static int __marshal__model2adapter(void* _mfc, void* _spec) } if (mfc->signal_value_binary) { for (uint32_t si = 0; si < mfc->signal_count; si++) { + if (mfc->signal_value_binary_reset_called[si] == false) { + /* Force size to 0. + Expected operation is: read, reset, write (append). + If reset is not called, i.e. the Model does not consume + _this_ signal, then data will be echo'ed back. If more than + one model echoes data back, the SimBus may also echo back + and ever increasing about of data. */ + mfc->signal_value_binary_size[si] = 0; + } dse_buffer_append(&sm[si].signal->bin, &sm[si].signal->bin_size, &sm[si].signal->bin_buffer_size, mfc->signal_value_binary[si], mfc->signal_value_binary_size[si]); diff --git a/dse/modelc/controller/controller.h b/dse/modelc/controller/controller.h index 435926f..c206f5b 100644 --- a/dse/modelc/controller/controller.h +++ b/dse/modelc/controller/controller.h @@ -26,6 +26,7 @@ typedef struct ModelFunctionChannel { void** signal_value_binary; uint32_t* signal_value_binary_size; uint32_t* signal_value_binary_buffer_size; + bool* signal_value_binary_reset_called; } ModelFunctionChannel; diff --git a/dse/modelc/model.h b/dse/modelc/model.h index 691078b..4a25f28 100644 --- a/dse/modelc/model.h +++ b/dse/modelc/model.h @@ -218,10 +218,11 @@ typedef struct SignalVector { }; struct { void** binary; - uint32_t* length; /* Length of binary object. */ - uint32_t* buffer_size; /* Size of allocated buffer. */ + uint32_t* length; /* Length of binary object. */ + uint32_t* buffer_size; /* Size of allocated buffer. */ const char** mime_type; - void** ncodec; /* Network Codec objects. */ + void** ncodec; /* Network Codec objects. */ + bool* reset_called; /* Indicate that reset() was called. */ }; }; /* Helper functions. */ diff --git a/dse/modelc/model/model.c b/dse/modelc/model/model.c index 7676773..83206bd 100644 --- a/dse/modelc/model/model.c +++ b/dse/modelc/model/model.c @@ -50,6 +50,8 @@ void model_function_destroy(ModelFunction* model_function) free(_mfc->signal_value_binary_size); if (_mfc && _mfc->signal_value_binary_buffer_size) free(_mfc->signal_value_binary_buffer_size); + if (_mfc && _mfc->signal_value_binary_reset_called) + free(_mfc->signal_value_binary_reset_called); if (_mfc && _mfc->signal_names) { // fixme Unit tests suggest a double free here. // for (uint32_t _ = 0; _ < _mfc->signal_count; _++) @@ -290,6 +292,8 @@ int model_configure_channel( calloc(signal_list.length, sizeof(uint32_t)); mfc->signal_value_binary_buffer_size = calloc(signal_list.length, sizeof(uint32_t)); + mfc->signal_value_binary_reset_called = + calloc(signal_list.length, sizeof(bool)); /* Set the references to those vectors (in the Channel Desc).*/ channel_desc->vector_binary = mfc->signal_value_binary; channel_desc->vector_binary_size = mfc->signal_value_binary_size; diff --git a/dse/modelc/model/ncodec.c b/dse/modelc/model/ncodec.c index 10f31a9..6721ca4 100644 --- a/dse/modelc/model/ncodec.c +++ b/dse/modelc/model/ncodec.c @@ -96,6 +96,7 @@ static int stream_seek(NCODEC* nc, size_t pos, int op) /* Reset before stream_write has truncate effect. */ _s->pos = 0; _s->sv->length[_s->idx] = 0; + _s->sv->reset_called[_s->idx] = true; } else { return -EINVAL; } diff --git a/dse/modelc/model/signal.c b/dse/modelc/model/signal.c index 64b3144..4af053e 100644 --- a/dse/modelc/model/signal.c +++ b/dse/modelc/model/signal.c @@ -124,7 +124,9 @@ static int __binary_append( static int __binary_reset(SignalVector* sv, uint32_t index) { + /* Note, changes here must be duplicated in stream_seek(). */ sv->length[index] = 0; + sv->reset_called[index] = true; NCodecInstance* nc = sv->ncodec[index]; if (nc && nc->stream && nc->stream->seek) { @@ -198,6 +200,7 @@ static int _add_sv(void* _mfc, void* _sv_data) current_sv->binary = mfc->signal_value_binary; current_sv->length = mfc->signal_value_binary_size; current_sv->buffer_size = mfc->signal_value_binary_buffer_size; + current_sv->reset_called = mfc->signal_value_binary_reset_called; current_sv->append = __binary_append; current_sv->reset = __binary_reset; current_sv->release = __binary_release; diff --git a/tests/cmocka/CMakeLists.txt b/tests/cmocka/CMakeLists.txt index fd9328a..2111b61 100644 --- a/tests/cmocka/CMakeLists.txt +++ b/tests/cmocka/CMakeLists.txt @@ -204,6 +204,7 @@ install(TARGETS test_model_interface) install( FILES model/interface/annotations.yaml + model/interface/binary_stream_reset.yaml DESTINATION resources/model ) diff --git a/tests/cmocka/model/interface/binary_stream_reset.yaml b/tests/cmocka/model/interface/binary_stream_reset.yaml new file mode 100644 index 0000000..4114777 --- /dev/null +++ b/tests/cmocka/model/interface/binary_stream_reset.yaml @@ -0,0 +1,63 @@ +# Copyright 2024 Robert Bosch GmbH +# +# SPDX-License-Identifier: Apache-2.0 + +--- +kind: Stack +metadata: + name: binary_stack +spec: + connection: + transport: + redispubsub: + uri: redis://localhost:6379 + timeout: 60 + models: + - name: simbus + model: + name: simbus + channels: + - name: scalar_channel + expectedModelCount: 1 + - name: binary_channel + expectedModelCount: 1 + - name: binary_inst + uid: 42 + model: + name: Binary + channels: + - name: scalar_channel + alias: scalar + - name: binary_channel + alias: binary +--- +kind: Model +metadata: + name: simbus +--- +kind: SignalGroup +metadata: + name: scalar_channel + labels: + channel: scalar_channel +spec: + signals: + - signal: counter +--- +kind: SignalGroup +metadata: + name: binary_channel + labels: + channel: binary_channel + annotations: + vector_type: binary +spec: + signals: + - signal: message + annotations: + mime_type: 'application/octet-stream' + - signal: not_used + annotations: + mime_type: 'application/octet-stream' + + diff --git a/tests/cmocka/model/interface/test_model_api.c b/tests/cmocka/model/interface/test_model_api.c index eb42df9..737539e 100644 --- a/tests/cmocka/model/interface/test_model_api.c +++ b/tests/cmocka/model/interface/test_model_api.c @@ -169,6 +169,73 @@ void test_model_api__model_annotation(void** state) } +#define BINARY_INST_NAME "binary_inst" +#define BINARY_SIGNAL_MESSAGE 0 +#define BINARY_SIGNAL_NOT_USED 1 + +void test_model_api__binary_stream_reset(void** state) +{ + chdir("../../../../dse/modelc/build/_out/examples/binary"); + + const char* inst_names[] = { + BINARY_INST_NAME, + }; + char* argv[] = { + (char*)"test_model_api", + (char*)"--name=" BINARY_INST_NAME, + (char*)"--logger=5", // 1=debug, 5=QUIET (commit with 5!) + (char*)"../../../../../../tests/cmocka/build/_out/resources/model/binary_stream_reset.yaml", + (char*)"data/model.yaml", + }; + SimMock* mock = *state = simmock_alloc(inst_names, ARRAY_SIZE(inst_names)); + simmock_configure(mock, argv, ARRAY_SIZE(argv), ARRAY_SIZE(inst_names)); + ModelMock* model = simmock_find_model(mock, BINARY_INST_NAME); + simmock_load(mock); + simmock_load_model_check(model, true, true, true); + simmock_setup(mock, "scalar_channel", "binary_channel"); + + double counter = 42.0; + char buffer[100] = ""; + uint32_t len = 0; + + simmock_print_binary_signals(mock, LOG_DEBUG); + + /* T0 ... Tn */ + for (uint32_t i = 0; i < 5; i++) { + /* Do the check. */ + BinaryCheck b_checks[] = { + { .index = BINARY_SIGNAL_MESSAGE, + .buffer = (uint8_t*)buffer, + .len = len }, + /* Len should be 0, buffer would not matter. */ + { .index = BINARY_SIGNAL_NOT_USED, + .buffer = (uint8_t*)buffer, + .len = 0 }, + }; + simmock_binary_check( + mock, BINARY_INST_NAME, b_checks, ARRAY_SIZE(b_checks), NULL); + /* Inject some binary data, if reset is called, this will be ignored. */ + SignalVector* sv = mock->sv_network_tx; + char foobar[100] = "foobar"; + for (int j = 0; j < 2; j++) { + sv->reset(sv, j); + sv->append(sv, j, foobar, strlen(foobar) + 1); + } + /* Step the model. */ + assert_int_equal(simmock_step(mock, true), 0); + sv = model->sv_network; + assert_true(sv->reset_called[0]); + assert_false(sv->reset_called[1]); + assert_int_equal(sv->length[1], 0); + simmock_print_binary_signals(mock, LOG_DEBUG); + /* Set check conditions (for next step). */ + counter += 1.0; + snprintf(buffer, sizeof(buffer), "count is %d", (int)counter); + len = strlen(buffer) + 1; + } +} + + int run_model_api_tests(void) { void* s = test_setup; @@ -179,6 +246,7 @@ int run_model_api_tests(void) const struct CMUnitTest tests[] = { cmocka_unit_test_setup_teardown(test_model_api__model_index, s, t), cmocka_unit_test_setup_teardown(test_model_api__model_annotation, s, t), + cmocka_unit_test_setup_teardown(test_model_api__binary_stream_reset, s, t), }; return cmocka_run_group_tests_name("MODEL / API", tests, NULL, NULL); diff --git a/tests/cmocka/model/test_signal.c b/tests/cmocka/model/test_signal.c index 57d3824..ecb1d08 100644 --- a/tests/cmocka/model/test_signal.c +++ b/tests/cmocka/model/test_signal.c @@ -162,6 +162,7 @@ void test_signal__binary(void** state) assert_non_null(sv->binary); assert_non_null(sv->length); assert_non_null(sv->buffer_size); + assert_non_null(sv->reset_called); assert_non_null(sv->append); assert_non_null(sv->reset); assert_non_null(sv->release); @@ -185,6 +186,7 @@ void test_signal__binary(void** state) assert_null(sv->binary[i]); assert_int_equal(sv->length[i], 0); assert_int_equal(sv->buffer_size[i], 0); + assert_false(sv->reset_called[i]); /* Append to the value. */ sv->append(sv, i, test_val, test_val_len); assert_non_null(sv->binary[i]); @@ -192,7 +194,9 @@ void test_signal__binary(void** state) assert_int_equal(sv->length[i], test_val_len); assert_int_equal(sv->buffer_size[i], test_val_len); /* Reset the value. */ + assert_false(sv->reset_called[i]); sv->reset(sv, i); + assert_true(sv->reset_called[i]); assert_non_null(sv->binary[i]); assert_int_equal(sv->length[i], 0); assert_int_equal(sv->buffer_size[i], test_val_len);