Skip to content

Commit

Permalink
Fix segmented download of objects larger than 64 bits
Browse files Browse the repository at this point in the history
The notification callback was not being triggered for downloads of
objects larger than 64 bits. Also, the cached field of the SDO job was
never cleared which would cause downloads of large objects to always
fail once a small object had been downloaded.
  • Loading branch information
hefloryd committed Feb 9, 2024
1 parent d152e8d commit e203bea
Show file tree
Hide file tree
Showing 7 changed files with 55 additions and 29 deletions.
2 changes: 2 additions & 0 deletions src/co_main.c
Original file line number Diff line number Diff line change
Expand Up @@ -277,6 +277,7 @@ int co_sdo_read (
job->sdo.subindex = subindex;
job->sdo.data = data;
job->sdo.remain = size;
job->sdo.cached = false;
job->callback = co_job_callback;
job->timestamp = os_tick_current();
job->type = CO_JOB_SDO_READ;
Expand Down Expand Up @@ -306,6 +307,7 @@ int co_sdo_write (
job->sdo.subindex = subindex;
job->sdo.data = (uint8_t *)data;
job->sdo.remain = size;
job->sdo.cached = false;
job->callback = co_job_callback;
job->timestamp = os_tick_current();
job->type = CO_JOB_SDO_WRITE;
Expand Down
2 changes: 1 addition & 1 deletion src/co_od.c
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ static int co_subindex_equals (
return entry->subindex == subindex;
}

static void co_od_notify (
void co_od_notify (
co_net_t * net,
const co_obj_t * obj,
const co_entry_t * entry,
Expand Down
17 changes: 17 additions & 0 deletions src/co_od.h
Original file line number Diff line number Diff line change
Expand Up @@ -271,6 +271,23 @@ uint32_t co_od_set_value (
uint8_t subindex,
uint64_t value);

/**
* Trigger notification callback
*
* This functions triggers the notification callback of the subindex,
* if any.
*
* @param net network handle
* @param obj object descriptor
* @param entry entry descriptor
* @param subindex subindex
*/
void co_od_notify (
co_net_t * net,
const co_obj_t * obj,
const co_entry_t * entry,
uint8_t subindex);

#ifdef __cplusplus
}
#endif
Expand Down
56 changes: 31 additions & 25 deletions src/co_sdo_server.c
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,7 @@ static int co_sdo_rx_upload_init_req (
job->type = CO_JOB_SDO_UPLOAD;
job->sdo.index = co_fetch_uint16 (&data[1]);
job->sdo.subindex = data[3];
job->sdo.cached = false;
job->timestamp = os_tick_current();

/* Find requested object */
Expand Down Expand Up @@ -306,6 +307,7 @@ static int co_sdo_rx_download_init_req (
job->type = CO_JOB_SDO_DOWNLOAD;
job->sdo.index = co_fetch_uint16 (&data[1]);
job->sdo.subindex = data[3];
job->sdo.cached = false;
job->timestamp = os_tick_current();

/* Find requested object */
Expand Down Expand Up @@ -466,34 +468,34 @@ static int co_sdo_rx_download_seg_req (
/* Write complete */
job->type = CO_JOB_NONE;

if (job->sdo.cached)
/* Find requested object */
obj = co_obj_find (net, job->sdo.index);
if (obj == NULL)
{
/* Find requested object */
obj = co_obj_find (net, job->sdo.index);
if (obj == NULL)
{
co_sdo_abort (
net,
0x580 + net->node,
job->sdo.index,
job->sdo.subindex,
CO_SDO_ABORT_BAD_INDEX);
return -1;
}
co_sdo_abort (
net,
0x580 + net->node,
job->sdo.index,
job->sdo.subindex,
CO_SDO_ABORT_BAD_INDEX);
return -1;
}

/* Find requested subindex */
entry = co_entry_find (net, obj, job->sdo.subindex);
if (entry == NULL)
{
co_sdo_abort (
net,
0x580 + net->node,
job->sdo.index,
job->sdo.subindex,
CO_SDO_ABORT_BAD_SUBINDEX);
return -1;
}
/* Find requested subindex */
entry = co_entry_find (net, obj, job->sdo.subindex);
if (entry == NULL)
{
co_sdo_abort (
net,
0x580 + net->node,
job->sdo.index,
job->sdo.subindex,
CO_SDO_ABORT_BAD_SUBINDEX);
return -1;
}

if (job->sdo.cached)
{
/* Atomically set value */
abort =
co_od_set_value (net, obj, entry, job->sdo.subindex, job->sdo.value);
Expand All @@ -508,6 +510,10 @@ static int co_sdo_rx_download_seg_req (
return -1;
}
}
else
{
co_od_notify (net, obj, entry, job->sdo.subindex);
}
}

/* Segmented response */
Expand Down
4 changes: 2 additions & 2 deletions test/test_sdo_client.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ TEST_F (SdoClientTest, ExpeditedUpload)

TEST_F (SdoClientTest, ExpeditedDownload)
{
co_job_t job;
co_job_t job{};
uint16_t value = 0;

uint8_t expected[][8] = {
Expand Down Expand Up @@ -136,7 +136,7 @@ TEST_F (SdoClientTest, SegmentedUpload)

TEST_F (SdoClientTest, SegmentedDownload)
{
co_job_t job;
co_job_t job{};
const char * s = "hello world";

uint8_t expected[][8] = {
Expand Down
1 change: 1 addition & 0 deletions test/test_sdo_server.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,7 @@ TEST_F (SdoServerTest, SegmentedDownload)
}

EXPECT_STREQ ("new slave name", name1009);
EXPECT_EQ (1u, cb_notify_calls);
}

TEST_F (SdoServerTest, SegmentedDownloadCached)
Expand Down
2 changes: 1 addition & 1 deletion test/test_util.h
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ class TestBase : public ::testing::Test

char name1009[20] = {0};
co_entry_t OD1009[1] = {
{0, OD_RW, DTYPE_VISIBLE_STRING, 8 * sizeof (name1009), 0, name1009},
{0, OD_NOTIFY | OD_RW, DTYPE_VISIBLE_STRING, 8 * sizeof (name1009), 0, name1009},
};

char name100A[20];
Expand Down

0 comments on commit e203bea

Please sign in to comment.