Skip to content

Commit

Permalink
modules: Update buffer size and name
Browse files Browse the repository at this point in the history
Rename 'string' payload message type to 'buffer'.

The payload exclusively carries CBOR-encoded data, making 'buffer' a
more accurate term than 'string' since the data is
not treated as a string.

Also increase the size of the buffer as we have seen indications that
in might be too small in some cases.

Signed-off-by: Simen S. Røstad <[email protected]>
  • Loading branch information
simensrostad committed Aug 28, 2024
1 parent 633eef1 commit 1669a36
Show file tree
Hide file tree
Showing 13 changed files with 30 additions and 28 deletions.
9 changes: 5 additions & 4 deletions app/Kconfig
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,12 @@

menu "Thingy Out-of-Box Application"

config APP_PAYLOAD_CHANNEL_STRING_MAX_SIZE
int "Payload maximum string size"
default 100
config APP_PAYLOAD_CHANNEL_BUFFER_MAX_SIZE
int "Payload maximum buffer size"
default 128
help
Maximum size of the string included messages that are sent over the payload channel.
Maximum size of the buffer sent over the payload channel.
Contains encoded CBOR data sampled and encoded in the various modules.

rsource "src/modules/trigger/Kconfig.trigger"
rsource "src/modules/battery/Kconfig.battery"
Expand Down
4 changes: 2 additions & 2 deletions app/src/common/message_channel.h
Original file line number Diff line number Diff line change
Expand Up @@ -46,8 +46,8 @@ extern "C" {
} while (0)

struct payload {
char string[CONFIG_APP_PAYLOAD_CHANNEL_STRING_MAX_SIZE];
size_t string_len;
uint8_t buffer[CONFIG_APP_PAYLOAD_CHANNEL_BUFFER_MAX_SIZE];
size_t buffer_len;
};

#define MSG_TO_PAYLOAD(_msg) ((struct payload *)_msg)
Expand Down
4 changes: 2 additions & 2 deletions app/src/modules/battery/battery.c
Original file line number Diff line number Diff line change
Expand Up @@ -243,8 +243,8 @@ static void sample(int64_t *ref_time)
bat_object.voltage_m.vf = voltage;
bat_object.temperature_m.vf = temp;

err = cbor_encode_bat_object(payload.string, sizeof(payload.string),
&bat_object, &payload.string_len);
err = cbor_encode_bat_object(payload.buffer, sizeof(payload.buffer),
&bat_object, &payload.buffer_len);
if (err) {
LOG_ERR("Failed to encode env object, error: %d", err);
SEND_FATAL_ERROR();
Expand Down
4 changes: 2 additions & 2 deletions app/src/modules/environmental/environmental.c
Original file line number Diff line number Diff line change
Expand Up @@ -167,8 +167,8 @@ static void sample(void)
env_obj.pressure_m.vf = sensor_value_to_double(&press) / 100;
env_obj.iaq_m.vi = iaq.val1;

ret = cbor_encode_env_object(payload.string, sizeof(payload.string),
&env_obj, &payload.string_len);
ret = cbor_encode_env_object(payload.buffer, sizeof(payload.buffer),
&env_obj, &payload.buffer_len);
if (ret) {
LOG_ERR("Failed to encode env object, error: %d", ret);
SEND_FATAL_ERROR();
Expand Down
4 changes: 2 additions & 2 deletions app/src/modules/network/network.c
Original file line number Diff line number Diff line change
Expand Up @@ -186,8 +186,8 @@ static void sample_network_quality(void)
LOG_DBG("Energy Estimate: %d", conn_info_obj.energy_estimate_m.vi);
LOG_DBG("RSRP: %d dBm", conn_info_obj.rsrp_m.vi);

ret = cbor_encode_conn_info_object(payload.string, sizeof(payload.string),
&conn_info_obj, &payload.string_len);
ret = cbor_encode_conn_info_object(payload.buffer, sizeof(payload.buffer),
&conn_info_obj, &payload.buffer_len);
if (ret) {
LOG_ERR("Failed to encode conn info object, error: %d", ret);
SEND_FATAL_ERROR();
Expand Down
4 changes: 2 additions & 2 deletions app/src/modules/transport/transport.c
Original file line number Diff line number Diff line change
Expand Up @@ -405,9 +405,9 @@ static void state_connected_ready_run(void *o)
int err;
struct payload *payload = MSG_TO_PAYLOAD(state_object->msg_buf);

LOG_HEXDUMP_DBG(payload->string, MIN(payload->string_len, 32), "Payload");
LOG_HEXDUMP_DBG(payload->buffer, MIN(payload->buffer_len, 32), "Payload");

err = nrf_cloud_coap_bytes_send(payload->string, payload->string_len, false);
err = nrf_cloud_coap_bytes_send(payload->buffer, payload->buffer_len, false);
if (err == -EACCES) {

/* Not connected, retry connection */
Expand Down
2 changes: 1 addition & 1 deletion tests/module/environmental/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ zephyr_include_directories(../../../app/src/common)
target_link_options(app PRIVATE --whole-archive)
# Options that cannot be passed through Kconfig fragments
target_compile_definitions(app PRIVATE
-DCONFIG_APP_PAYLOAD_CHANNEL_STRING_MAX_SIZE=100
-DCONFIG_APP_PAYLOAD_CHANNEL_BUFFER_MAX_SIZE=100
-DCONFIG_APP_ENVIRONMENTAL_LOG_LEVEL=4
-DCONFIG_APP_ENVIRONMENTAL_THREAD_STACK_SIZE=1024
-DCONFIG_APP_ENVIRONMENTAL_MESSAGE_QUEUE_SIZE=5
Expand Down
3 changes: 2 additions & 1 deletion tests/module/environmental/src/main.c
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,8 @@ void wait_for_and_decode_payload(struct env_object *env_object)
}

/* decode payload */
cbor_decode_env_object(received_payload.string, received_payload.string_len, env_object, NULL);
cbor_decode_env_object(received_payload.buffer,
received_payload.buffer_len, env_object, NULL);
if (err != ZCBOR_SUCCESS) {
LOG_ERR("Failed to decode payload");
TEST_FAIL();
Expand Down
2 changes: 1 addition & 1 deletion tests/module/network/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ zephyr_include_directories(../../../app/src/common)
target_link_options(app PRIVATE --whole-archive)
# Options that cannot be passed through Kconfig fragments
target_compile_definitions(app PRIVATE
-DCONFIG_APP_PAYLOAD_CHANNEL_STRING_MAX_SIZE=100
-DCONFIG_APP_PAYLOAD_CHANNEL_BUFFER_MAX_SIZE=100
-DCONFIG_APP_NETWORK_LOG_LEVEL=4
-DCONFIG_APP_NETWORK_THREAD_STACK_SIZE=1024
-DCONFIG_APP_NETWORK_MESSAGE_QUEUE_SIZE=5
Expand Down
2 changes: 1 addition & 1 deletion tests/module/network/src/main.c
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ static void wait_for_and_decode_payload(struct conn_info_object *conn_info_obj)
}

/* decode payload */
cbor_decode_conn_info_object(payload.string, payload.string_len, conn_info_obj, NULL);
cbor_decode_conn_info_object(payload.buffer, payload.buffer_len, conn_info_obj, NULL);
if (err != ZCBOR_SUCCESS) {
LOG_ERR("Failed to decode payload");
TEST_FAIL();
Expand Down
2 changes: 1 addition & 1 deletion tests/module/transport/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ target_link_options(app PRIVATE --whole-archive)

# Options that cannot be passed through Kconfig fragments
target_compile_definitions(app PRIVATE
-DCONFIG_APP_PAYLOAD_CHANNEL_STRING_MAX_SIZE=100
-DCONFIG_APP_PAYLOAD_CHANNEL_BUFFER_MAX_SIZE=100
-DCONFIG_APP_TRANSPORT_LOG_LEVEL=0
-DCONFIG_APP_TRANSPORT_THREAD_STACK_SIZE=2048
-DCONFIG_APP_TRANSPORT_WORKQUEUE_STACK_SIZE=4096
Expand Down
16 changes: 8 additions & 8 deletions tests/module/transport/src/transport_module_test.c
Original file line number Diff line number Diff line change
Expand Up @@ -109,8 +109,8 @@ void test_transition_disconnected_connected_ready(void)
void test_sending_payload(void)
{
struct payload payload = {
.string = "test",
.string_len = sizeof(payload.string) - 1,
.buffer = "test",
.buffer_len = sizeof(payload.buffer) - 1,
};

zbus_chan_pub(&PAYLOAD_CHAN, &payload, K_NO_WAIT);
Expand All @@ -120,8 +120,8 @@ void test_sending_payload(void)

TEST_ASSERT_EQUAL(1, nrf_cloud_coap_bytes_send_fake.call_count);
TEST_ASSERT_EQUAL(0, strncmp(nrf_cloud_coap_bytes_send_fake.arg0_val,
payload.string, payload.string_len));
TEST_ASSERT_EQUAL(payload.string_len, nrf_cloud_coap_bytes_send_fake.arg1_val);
payload.buffer, payload.buffer_len));
TEST_ASSERT_EQUAL(payload.buffer_len, nrf_cloud_coap_bytes_send_fake.arg1_val);
}

void test_connected_ready_to_paused(void)
Expand All @@ -143,8 +143,8 @@ void test_connected_paused_to_ready_send_payload(void)
int err;
enum network_status status = NETWORK_CONNECTED;
struct payload payload = {
.string = "Another test",
.string_len = sizeof(payload.string) - 1,
.buffer = "Another test",
.buffer_len = sizeof(payload.buffer) - 1,
};

/* Reset call count */
Expand All @@ -165,8 +165,8 @@ void test_connected_paused_to_ready_send_payload(void)

TEST_ASSERT_EQUAL(1, nrf_cloud_coap_bytes_send_fake.call_count);
TEST_ASSERT_EQUAL(0, strncmp(nrf_cloud_coap_bytes_send_fake.arg0_val,
payload.string, payload.string_len));
TEST_ASSERT_EQUAL(payload.string_len, nrf_cloud_coap_bytes_send_fake.arg1_val);
payload.buffer, payload.buffer_len));
TEST_ASSERT_EQUAL(payload.buffer_len, nrf_cloud_coap_bytes_send_fake.arg1_val);
}

/* This is required to be added to each test. That is because unity's
Expand Down
2 changes: 1 addition & 1 deletion tests/module/trigger/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ zephyr_include_directories(../../../app/src/common)

# Options that cannot be passed through Kconfig fragments
target_compile_definitions(app PRIVATE
-DCONFIG_APP_PAYLOAD_CHANNEL_STRING_MAX_SIZE=100
-DCONFIG_APP_PAYLOAD_CHANNEL_BUFFER_MAX_SIZE=100
-DCONFIG_APP_TRIGGER_LOG_LEVEL=4
-DCONFIG_APP_TRIGGER_THREAD_STACK_SIZE=1024
-DCONFIG_APP_TRIGGER_MESSAGE_QUEUE_SIZE=5
Expand Down

0 comments on commit 1669a36

Please sign in to comment.