Prevent echo of binary data when reset() is not called on a signal.

Signed-off-by: Rule Timothy (VM/EMT3) <[email protected]>

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]);
             }

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]);

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;
 
 

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. */

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;

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;
         }

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;

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
 )

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'
+
+

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);

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,14 +186,17 @@ 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]);
         assert_string_equal((char*)sv->binary[i], test_val);
         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);