RDKB-65730: Integrate Dynamic Table Support - MsgPack support and L2 tests

[yogeswaransky]

No description provided.

[Copilot]

Pull request overview

This PR extends the Telemetry 2.0 profile parser to support dynamic table (“dataModelTable”) configuration in the MsgPack profile flow, aligning MsgPack behavior more closely with the existing JSON dynamic-table parsing path.

Changes:

• Added MsgPack dynamic-table parser helper (parseDataModelTableParamsMsgpack) under ENABLE_DYNAMIC_TABLE_SUPPORT.
• Implemented MsgPack handling for type == "dataModelTable" in addParameterMsgpack_marker_config, including optional index expansion and dynamic-table structure parsing.
• Introduced a msgpack_add_param label to allow the no-index dynamic-table case to fall through into the existing parameter-add path.

Suppressed comments (1)

source/t2parser/t2parser.c:2351

• referenceStr ownership differs between root vs nested calls (root stores it in currentTable->reference). On these failure paths it is currently freed only when parentTable == NULL, which can leave currentTable->reference dangling (and later double-freed in freeDataModelTable) and also leaks referenceStr for nested calls. Free referenceStr only for nested calls on error paths.

    if (buildFullPath(currentPath, parentPath, referenceStr) != 0)
    {
        T2Error("Failed to build current path\n");
        if (!parentTable) free(referenceStr);
        return T2ERROR_FAILURE;

[Copilot]

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 1 comment.

Suppressed comments (3)

source/t2parser/t2parser.c:2764

• Same as above: snprintf truncation/failure when building basePathWithIndex is logged but addParameter() is still called with a potentially truncated path. Skip the entry if snprintf fails/truncates.

                            int written = snprintf(basePathWithIndex, sizeof(basePathWithIndex), "%s%d.", basePath, val);
                            if (written < 0 || (size_t)written >= sizeof(basePathWithIndex))
                            {
                                T2Error("%s: snprintf truncated or failed while building path: '%s'\n", __FUNCTION__, basePathWithIndex);
                            }

source/t2parser/t2parser.c:2737

• When building basePathWithIndex, snprintf truncation/failure is logged but execution still calls addParameter() with a potentially truncated path. That can add incorrect parameter names into the profile. Treat truncation as an error and skip the index entry.

This issue also appears on line 2760 of the same file.

                                int written = snprintf(basePathWithIndex, sizeof(basePathWithIndex), "%s%d.", basePath, k);
                                if (written < 0 || (size_t)written >= sizeof(basePathWithIndex))
                                {
                                    T2Error("%s: snprintf truncated or failed while building path: '%s'\n", __FUNCTION__, basePathWithIndex);
                                }

source/t2parser/t2parser.c:2678

• This PR adds a new MsgPack-specific dataModelTable parsing flow, but there don’t appear to be any unit tests covering MsgPack profiles with dataModelTable (existing dynamic table tests are JSON-based). Please add/extend gtest coverage to include (at minimum) dataModelTable with and without index in MsgPack, plus a nested table case, to prevent regressions in addParameterMsgpack_marker_config()/parseDataModelTableParamsMsgpack().

            T2Debug("Processing dataModelTable in MsgPack profile\n");
            msgpack_object *mpBaseRef = msgpack_get_map_value(Parameter_array_map, "reference");
            if (mpBaseRef)
            {
                char basePath[256] = "";

[Copilot]

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.

Suppressed comments (3)

source/t2parser/t2parser.c:2351

• On buildFullPath / wildcard-path failures, the code frees referenceStr when parsing the root table. But for the root table currentTable->reference points to referenceStr, so freeing it leaves a dangling pointer in profile->dataModelTableList and can crash later (e.g., report generation / cleanup). Clean up the table entry from the vector on failure instead of freeing the owned string.

    if (buildFullPath(currentPath, parentPath, referenceStr) != 0)
    {
        T2Error("Failed to build current path\n");
        if (!parentTable)
        {

test/run_l2.sh:58

• mock_table_provider is started in the background but cleanup is only a best-effort kill at the end of the test block. If the script exits early (e.g., gcc/pytest failure, SIGINT), the provider can be left running and interfere with subsequent test runs. Add a trap-based cleanup and fail fast if compilation/startup fails.

# Compile mock table provider for dataModelTable L2 tests
gcc -o test/functional-tests/tests/mock_table_provider test/functional-tests/tests/mock_table_provider.c \
    -I/usr/local/include -I/usr/local/include/rbus \
    -L/usr/local/lib -lrbus -lrbuscore -lrtMessage -lmsgpackc

# Start mock table provider in background (provides Device.X_T2TEST_Table.AccessPoint.{1,2,3}.*)
test/functional-tests/tests/mock_table_provider &
MOCK_TABLE_PROVIDER_PID=$!
sleep 2

final_result=0
# removing --exitfirst flag as it is causing the test to exit after first failure
pytest -v --json-report --json-report-summary --json-report-file $RESULT_DIR/runs_as_daemon.json test/functional-tests/tests/test_runs_as_daemon.py || final_result=1
pytest -v --json-report --json-report-summary --json-report-file $RESULT_DIR/bootup_sequence.json test/functional-tests/tests/test_bootup_sequence.py || final_result=1
pytest -v --json-report --json-report-summary --json-report-file $RESULT_DIR/xconf_communications.json test/functional-tests/tests/test_xconf_communications.py || final_result=1
pytest -v --json-report --json-report-summary --json-report-file $RESULT_DIR/msg_packet.json test/functional-tests/tests/test_multiprofile_msgpacket.py || final_result=1
pytest -v --json-report --json-report-summary --json-report-file $RESULT_DIR/datamodeltable.json test/functional-tests/tests/test_datamodeltable.py || final_result=1

# Stop mock table provider
kill $MOCK_TABLE_PROVIDER_PID 2>/dev/null

source/t2parser/t2parser.c:2408

• param->reference is used unconditionally in buildFullPath(...), but msgpack_strdup can fail and return NULL (OOM). That would turn into a NULL dereference in buildFullPath (which logs an error but still dereferences reference to compute length/indexing in some code paths). Add a NULL check and free param early.

            param->reference = msgpack_strdup(mpParamRef);
            char fullPath[MAX_PATH_LENGTH];
            if (buildFullPath(fullPath, pathWithWildcard, param->reference) != 0)

[Copilot]

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated no new comments.

Suppressed comments (6)

test/functional-tests/tests/mock_table_provider.c:112

• In the wildcard query path, current is set to next after rbusProperty_Release(next), so the next loop iteration appends to a released property (use-after-free / corrupted property chain). Avoid using next after releasing it; typically the appended property should remain owned by the list until the RBUS framework releases the head property.

                rbusProperty_t next;
                rbusProperty_Init(&next, paramNames[i], val);
                rbusProperty_Append(current, next);
                rbusProperty_Release(next);
                current = next;

source/t2parser/t2parser.c:2343

• referenceStr is reallocated for nested tables without a NULL-check, and the error path frees referenceStr when !parentTable even though ownership was already transferred to currentTable->reference. This can lead to NULL dereference in buildFullPath and a dangling currentTable->reference (double-free later).

        currentTable = parentTable;
        free(referenceStr);
        referenceStr = msgpack_strdup(mpReference);
    }

source/t2parser/t2parser.c:2360

• On the wildcard-path snprintf failure, the error path frees referenceStr when !parentTable, but in the root-table case referenceStr is stored in currentTable->reference. Freeing it here leaves a dangling pointer in the table list and can cause a double-free during freeDataModelTable.

    if ((size_t)snprintf(pathWithWildcard, sizeof(pathWithWildcard), "%s*.", currentPath) >= sizeof(pathWithWildcard))
    {
        T2Error("Path with wildcard exceeded buffer size\n");

test/run_l2.sh:47

• The mock table provider is started in the background without any guarantee it will be stopped if the script exits early (e.g., gcc failure, interrupted CI job). Also, the gcc invocation isn’t checked, so the script may continue with a missing/old binary. Add an explicit build check and an EXIT trap that always terminates the background provider.

# Compile mock table provider for dataModelTable L2 tests
gcc -o test/functional-tests/tests/mock_table_provider test/functional-tests/tests/mock_table_provider.c \
    -I/usr/local/include -I/usr/local/include/rbus \
    -L/usr/local/lib -lrbus -lrbuscore -lrtMessage -lmsgpackc

test/run_l2.sh:58

• After introducing cleanup_mock_table_provider, use it here instead of a raw kill so the process is waited on and the logic stays in one place (and matches the EXIT trap).

# Stop mock table provider
kill $MOCK_TABLE_PROVIDER_PID 2>/dev/null

source/t2parser/t2parser.c:2675

• MsgPack dataModelTable parsing is now implemented, but there are no unit tests covering this MsgPack-only path (existing dynamic table tests exercise the JSON flow). Add gtests under source/test/t2parser/ that build a MsgPack profile containing dataModelTable (wildcard, explicit index, and nested) and assert profile->dataModelTableList and paramList contents.

        else if(0 == msgpack_strcmp(Parameter_type_str, "dataModelTable"))
        {
#ifdef ENABLE_DYNAMIC_TABLE_SUPPORT
            T2Debug("Processing dataModelTable in MsgPack profile\n");
            msgpack_object *mpBaseRef = msgpack_get_map_value(Parameter_array_map, "reference");

[Copilot]

Pull request overview

Copilot reviewed 9 out of 9 changed files in this pull request and generated no new comments.

Suppressed comments (8)

test/functional-tests/tests/mock_table_provider.c:112

• next is appended to the RBUS property list and then immediately released, but current is set to next afterwards. This breaks the property chain and can become a use-after-free when RBUS later traverses the list (the main codebase releases only the head property after iteration; see source/ccspinterface/rbusInterface.c:391-394).

                rbusProperty_t next;
                rbusProperty_Init(&next, paramNames[i], val);
                rbusProperty_Append(current, next);
                rbusProperty_Release(next);
                current = next;

source/t2parser/t2parser.c:2351

• On the buildFullPath error path, referenceStr is freed when parentTable is NULL, but in that case the pointer is owned by currentTable->reference (stored earlier). This can leave currentTable->reference dangling inside profile->dataModelTableList and cause a later free/crash.

    if (buildFullPath(currentPath, parentPath, referenceStr) != 0)
    {
        T2Error("Failed to build current path\n");
        if (!parentTable)
        {

source/t2parser/t2parser.c:2362

• On the wildcard snprintf overflow path, referenceStr is freed only when parentTable is NULL. For nested table parsing (parentTable != NULL), referenceStr is newly allocated and will leak on this error path; for root parsing, freeing it can again leave currentTable->reference dangling.

    if ((size_t)snprintf(pathWithWildcard, sizeof(pathWithWildcard), "%s*.", currentPath) >= sizeof(pathWithWildcard))
    {
        T2Error("Path with wildcard exceeded buffer size\n");
        if (!parentTable)
        {

test/functional-tests/tests/test_datamodeltable.py:38

• subprocess and dt are imported but never used, which adds noise and can mask real unused-import issues.

import subprocess
from time import sleep
from datetime import datetime as dt
import pytest

test/run_l2.sh:58

• The mock table provider is started in the background but only killed at the end without a trap or wait. If this script exits early (pytest error, SIGINT/TERM, etc.), the provider can keep running and interfere with later runs; kill without wait can also leave a zombie process. Also, the mock provider compile step is not checked for failure, so tests can run against a missing binary.

# Start mock table provider in background (provides Device.X_T2TEST_Table.AccessPoint.{1,2,3}.*)
test/functional-tests/tests/mock_table_provider &
MOCK_TABLE_PROVIDER_PID=$!
sleep 2

build_inside_container.sh:34

• This script now always enables dynamic table support, which makes the feature effectively non-optional for the container build and prevents running CI scenarios against a feature-disabled build (the L2 workflow uses this script). Consider gating this behind an environment variable so the default build remains representative.

./configure --prefix=${INSTALL_DIR} --enable-rdkcertselector=yes --enable-dynamic-table-support=yes && make && make install

test/functional-tests/tests/test_datamodeltable.py:26

• The scenario list claims the tests validate report JSON structure / row contents, but the assertions currently only check logs and that the process didn’t crash. Updating the scenario descriptions will avoid misleading future maintainers.

This issue also appears on line 35 of the same file.

1. Push a profile with dataModelTable (explicit index) -> verify report JSON structure
2. Push a profile with dataModelTable (wildcard) -> verify all rows appear in report
3. Push a profile with dataModelTable while a reporting cycle is active -> verify no crash

source/t2parser/t2parser.c:2681

• MsgPack dataModelTable support is newly added here, but there don’t appear to be any unit tests exercising the MsgPack profile parsing path (existing parser tests cover JSON/dynamic-table parsing only). Adding a focused unit test would help prevent regressions in the MsgPack flow.

            T2Debug("Processing dataModelTable in MsgPack profile\n");
            msgpack_object *mpBaseRef = msgpack_get_map_value(Parameter_array_map, "reference");
            if (mpBaseRef)
            {
                char basePath[256] = "";
                char *baseRefStr = msgpack_strdup(mpBaseRef);
                if (baseRefStr)
                {

[Copilot]

Pull request overview

Copilot reviewed 9 out of 9 changed files in this pull request and generated 1 comment.

Suppressed comments (11)

test/functional-tests/tests/mock_table_provider.c:113

• tableGetHandler appends a new property and then releases it, but still assigns current = next afterwards. If rbusProperty_Release decrements the refcount/free's the property (as the name implies), this makes current a dangling pointer on the next iteration.

                rbusProperty_t next;
                rbusProperty_Init(&next, paramNames[i], val);
                rbusProperty_Append(current, next);
                rbusProperty_Release(next);
                current = next;
            }

test/run_l2.sh:42

• The gcc build steps are not checked for failure. If compilation/linking fails, the script will continue and later steps will run with missing binaries, producing misleading test failures.

gcc test/functional-tests/tests/app.c -o test/functional-tests/tests/t2_app -ltelemetry_msgsender -lt2utils

# Compile mock table provider for dataModelTable L2 tests
gcc -o test/functional-tests/tests/mock_table_provider test/functional-tests/tests/mock_table_provider.c \
    -I/usr/local/include -I/usr/local/include/rbus \
    -L/usr/local/lib -lrbus -lrbuscore -lrtMessage -lmsgpackc

test/run_l2.sh:48

• The mock table provider is started in the background but there is no EXIT/INT/TERM trap to guarantee cleanup. If the script is interrupted (or exits early in the future), the provider can be left running and interfere with subsequent runs.

# Start mock table provider in background (provides Device.X_T2TEST_Table.AccessPoint.{1,2,3}.*)
test/functional-tests/tests/mock_table_provider &
MOCK_TABLE_PROVIDER_PID=$!
sleep 2

test/run_l2.sh:58

• After killing the background mock provider, the script does not wait for it to exit. This can leave a zombie process until the parent shell exits, and also increases flakiness if a subsequent step expects the provider to be gone.

# Stop mock table provider
kill $MOCK_TABLE_PROVIDER_PID 2>/dev/null

test/functional-tests/tests/mock_table_provider.c:9

• The file header comment claims it registers Device.WiFi.AccessPoint.{1,2,3}., but the implementation actually registers under Device.X_T2TEST_Table.AccessPoint.. This mismatch makes the test setup harder to understand and debug.

/*
 * Mock rbus table provider for L2 testing of dataModelTable feature.
 * Registers Device.WiFi.AccessPoint.{1,2,3}.{SSID,Status,Enable} as
 * indexed table parameters accessible via rbus.
 *

test/functional-tests/tests/mock_table_provider.c:197

• The main loop never checks for termination requested by the signal handler. If exitHandler is changed to set a termination flag (to remain signal-safe), main() should exit the loop and run RBUS cleanup before returning.

    while (1)
    {
        sleep(5);
    }

    return 0;
}

source/t2parser/t2parser.c:2685

• MsgPack support for dataModelTable is added here, but there are no unit tests covering the new MsgPack parsing flow (parseDataModelTableParamsMsgpack / dataModelTable branch in addParameterMsgpack_marker_config). This is a regression risk because the JSON path already has dedicated tests under source/test/t2parser/ but the MsgPack path does not.

        else if(0 == msgpack_strcmp(Parameter_type_str, "dataModelTable"))
        {
#ifdef ENABLE_DYNAMIC_TABLE_SUPPORT
            T2Debug("Processing dataModelTable in MsgPack profile\n");
            msgpack_object *mpBaseRef = msgpack_get_map_value(Parameter_array_map, "reference");
            if (mpBaseRef)
            {
                char basePath[256] = "";

schemas/t2_reportProfileSchema.schema.json:125

• The schema description says index values must be 0-255, but the regex pattern currently allows any size integer. Either enforce the 0-255 bound in the schema or soften the wording to avoid implying validation that the schema does not perform.

                "index": {
                    "type": "string",
                    "pattern": "^(\\d+(-\\d+)?)(,(\\d+(-\\d+)?))*$",
                    "description": "Optional. Comma-separated row indices or ranges to collect. Examples: '1' (single row), '1-4' (range), '1,3,5' (list), '1-2,5,7-9' (mixed). Values must be 0-255. If omitted, all available rows are collected via wildcard query."
                },

build_inside_container.sh:34

• build_inside_container.sh now forces --enable-dynamic-table-support=yes, which contradicts the documented default (disabled) and can unintentionally change the behavior of developer builds that rely on this script. Consider making it configurable via an environment variable and defaulting to 'no'.

./configure --prefix=${INSTALL_DIR} --enable-rdkcertselector=yes --enable-dynamic-table-support=yes && make && make install

test/functional-tests/tests/test_datamodeltable.py:33

• The module docstring claims scenarios 1 and 2 verify the report JSON structure/rows, but the tests only assert on log contents and that telemetry2_0 is still running. Either add assertions that parse the emitted report content, or update the scenario descriptions so they match what is actually being validated.

"""
L2 Integration Tests for dataModelTable feature (RDKB-65730 / AC4)

Scenarios:
1. Push a profile with dataModelTable (explicit index) -> verify report JSON structure
2. Push a profile with dataModelTable (wildcard) -> verify all rows appear in report
3. Push a profile with dataModelTable while a reporting cycle is active -> verify no crash
4. Build with feature disabled -> push same profile -> verify silently ignored

Prerequisites:
- mock_table_provider must be compiled and running in the container
  (provides Device.X_T2TEST_Table.AccessPoint.{1,2,3}.{SSID,Status,Enable})
- telemetry2_0 must be built with --enable-dynamic-table-support=yes
"""

test/functional-tests/tests/test_datamodeltable.py:177

• report_log is assigned but never used. If the intent is to validate that a report completed, add an assertion on the log content; otherwise remove the unused assignment to avoid misleading readers.

    # Verify report completed (look for report generation log)
    report_log = grep_T2logs("Report sent successfully")
    # Even if report wasn't sent (mock may not accept), no crash is the key requirement

[yogeswaransky]

Copilot review comments were addressed

[Copilot]

Pull request overview

Copilot reviewed 9 out of 9 changed files in this pull request and generated no new comments.

Suppressed comments (4)

test/functional-tests/tests/mock_table_provider.c:112

• In the wildcard handler, the code appends a new rbusProperty and then immediately releases it but still assigns current = next. That leaves current pointing at a released property, which can cause undefined behavior on the next iteration when appending again. Use the same linked-list pattern used elsewhere in the repo (rbusProperty_SetNext + Release) so current always refers to the live node in the list.

                rbusProperty_t next;
                rbusProperty_Init(&next, paramNames[i], val);
                rbusProperty_Append(current, next);
                rbusProperty_Release(next);
                current = next;

test/run_l2.sh:58

• The mock_table_provider background process is only killed at the end of the script; if the script exits early (compile error, interruption), the provider can be left running and interfere with subsequent tests. Also, gcc failures are not checked, so the test run may proceed with a missing binary.

# Compile mock table provider for dataModelTable L2 tests
gcc -o test/functional-tests/tests/mock_table_provider test/functional-tests/tests/mock_table_provider.c \
    -I/usr/local/include -I/usr/local/include/rbus \
    -L/usr/local/lib -lrbus -lrbuscore -lrtMessage -lmsgpackc

# Start mock table provider in background (provides Device.X_T2TEST_Table.AccessPoint.{1,2,3}.*)
test/functional-tests/tests/mock_table_provider &
MOCK_TABLE_PROVIDER_PID=$!
sleep 2

test/functional-tests/tests/mock_table_provider.c:4

• The file header comment claims the provider registers Device.WiFi.AccessPoint.* but the implementation actually registers Device.X_T2TEST_Table.AccessPoint.*. This mismatch makes the test dependency unclear when reading logs and profiles.

 * Mock rbus table provider for L2 testing of dataModelTable feature.
 * Registers Device.WiFi.AccessPoint.{1,2,3}.{SSID,Status,Enable} as
 * indexed table parameters accessible via rbus.

test/functional-tests/tests/test_datamodeltable.py:37

• Unused imports (subprocess and dt) add noise and can trigger lint/quality checks in some environments. Removing them keeps the test module minimal and avoids confusion about intended behavior.

import subprocess
from time import sleep
from datetime import datetime as dt

[Copilot]

Pull request overview

Copilot reviewed 9 out of 9 changed files in this pull request and generated no new comments.

Suppressed comments (5)

test/functional-tests/tests/test_datamodeltable.py:127

• This test claims it verifies all wildcard rows appear in the report, but it only checks that the base path appears in logs. Consider asserting against the cJSON Report log output and verifying that rows 1, 2, and 3 were encoded.

    # Verify wildcard data collection - all rows should be queried
    log_content = grep_T2logs("Device.X_T2TEST_Table.AccessPoint")
    assert "Device.X_T2TEST_Table.AccessPoint" in log_content, \
        "No evidence of wildcard table parameter collection in logs"

test/functional-tests/tests/mock_table_provider.c:4

• File header comment says the mock registers Device.WiFi.AccessPoint..., but the implementation actually registers Device.X_T2TEST_Table.AccessPoint... (TABLE_BASE). This mismatch makes it harder to understand what the provider serves during L2 runs.

 * Mock rbus table provider for L2 testing of dataModelTable feature.
 * Registers Device.WiFi.AccessPoint.{1,2,3}.{SSID,Status,Enable} as
 * indexed table parameters accessible via rbus.

test/functional-tests/tests/test_datamodeltable.py:82

• This test claims it verifies the generated report structure for explicit indexes, but it only checks that the table base path appears somewhere in logs (which could be from RBUS queries). To actually validate the feature, assert against the emitted cJSON Report content and confirm only rows 1 and 2 are present.

This issue also appears on line 124 of the same file.

    # Verify data collection happened for indexed params
    # T2 should query Device.X_T2TEST_Table.AccessPoint.1.SSID and .2.SSID
    log_content = grep_T2logs("Device.X_T2TEST_Table.AccessPoint")
    assert "Device.X_T2TEST_Table.AccessPoint" in log_content, \
        "No evidence of table parameter data collection in logs"

source/t2parser/t2parser.c:2692

• MsgPack dataModelTable parsing is newly implemented here, but there are no corresponding C unit tests covering MsgPack dynamic table profiles (e.g., validating profile->dataModelTableList, wildcard vs index behavior, and nested tables). Given the existing gtest suites under source/test/t2parser/, adding coverage would reduce regression risk for the new parsing path.

        else if(0 == msgpack_strcmp(Parameter_type_str, "dataModelTable"))
        {
#ifdef ENABLE_DYNAMIC_TABLE_SUPPORT
            T2Debug("Processing dataModelTable in MsgPack profile\n");
            msgpack_object *mpBaseRef = msgpack_get_map_value(Parameter_array_map, "reference");
            if (mpBaseRef)
            {
                char basePath[256] = "";

test/run_l2.sh:58

• The mock_table_provider is started in the background but is only killed at the end without an EXIT/INT/TERM trap, and the compilation/start steps are not checked. This can leave stray processes running if the script exits early (e.g., SIGINT/CI interruption) and can make failures harder to diagnose when gcc fails.

# Start mock table provider in background (provides Device.X_T2TEST_Table.AccessPoint.{1,2,3}.*)
test/functional-tests/tests/mock_table_provider &
MOCK_TABLE_PROVIDER_PID=$!
sleep 2

[Copilot]

Pull request overview

Copilot reviewed 9 out of 9 changed files in this pull request and generated no new comments.

Suppressed comments (7)

test/functional-tests/tests/test_datamodeltable.py:56

• The scenario docstring claims the report JSON structure/rows are verified, but the test only checks that logs mention the table path and that telemetry stays running. Add assertions against the generated report content (e.g., by grepping the existing "cJSON Report" log output for expected rows/fields) so the test actually validates the feature output, not just lack of crashes.

    Push a profile with dataModelTable (explicit index "1,2") and verify:
    - Profile is enabled successfully
    - T2 logs show data collection for indexed table parameters
    - Report contains structured array with row 1 and row 2 data only

test/run_l2.sh:42

• The mock_table_provider build step is not checked for failure; if gcc fails, the script still proceeds and the subsequent background start may run a stale binary or fail silently. Make the compile step fail-fast so the L2 run aborts with a clear signal.

gcc -o test/functional-tests/tests/mock_table_provider test/functional-tests/tests/mock_table_provider.c \
    -I/usr/local/include -I/usr/local/include/rbus \
    -L/usr/local/lib -lrbus -lrbuscore -lrtMessage -lmsgpackc

test/run_l2.sh:47

• The mock_table_provider is started in the background but only stopped at the end of the script; if the script is interrupted (or exits early later), the provider can be left running and interfere with subsequent runs. Add a trap to ensure it is always cleaned up.

# Start mock table provider in background (provides Device.X_T2TEST_Table.AccessPoint.{1,2,3}.*)
test/functional-tests/tests/mock_table_provider &
MOCK_TABLE_PROVIDER_PID=$!
sleep 2

test/functional-tests/tests/mock_table_provider.c:4

• Header comment describes registering Device.WiFi.AccessPoint.* but the implementation actually registers under Device.X_T2TEST_Table.AccessPoint.* (TABLE_BASE). Update the comment to match the actual namespace used by the mock provider.

 * Mock rbus table provider for L2 testing of dataModelTable feature.
 * Registers Device.WiFi.AccessPoint.{1,2,3}.{SSID,Status,Enable} as
 * indexed table parameters accessible via rbus.

source/t2parser/t2parser.c:2410

• The recursive parse of nested dataModelTable entries ignores the return value. If a nested table parse fails, the error is silently dropped and the profile may be left partially populated. Capture the return and log an explicit error (or propagate failure) for easier debugging.

        if (0 == msgpack_strcmp(mpType, "dataModelTable"))
        {
            // Recursive call for nested tables
            parseDataModelTableParamsMsgpack(profile, paramItem, pathWithWildcard, currentTable);
        }

source/t2parser/t2parser.c:2001

• msgpack_strcmp now calls strlen(str) without validating str; if any caller passes NULL this will crash. Add a NULL check for str to keep the helper safe (it already treats non-string msgpack objects as non-matches).

    size_t len = strlen(str);
    if (obj->via.str.size != len)
    {
        return (obj->via.str.size < len) ? -1 : 1;
    }

test/functional-tests/tests/test_datamodeltable.py:38

• This test module has unused imports (subprocess, datetime) which can cause lint failures and adds noise. Remove them if they are not needed.

This issue also appears on line 53 of the same file.

import subprocess
from time import sleep
from datetime import datetime as dt
import pytest