Skip to content

c-open: Fix cached status. Fix SDO 7B last packet. Add cb_notify event. #56

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
+201 −104
Open

c-open: Fix cached status. Fix SDO 7B last packet. Add cb_notify event. #56

Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
25f8d13
Add cb_notify event
Mar 6, 2023
d8c5af0
Fix SDO 7B last packet
Mar 6, 2023
a230742
Fix SDO cached status
Mar 6, 2023
5195c48
Remove duplicated defines
Mar 6, 2023
32bb524
Fix segmented download of objects larger than 64 bits
Mar 6, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion CMakeLists.txt
View file
Original file line number Diff line number Diff line change
@@ -106,7 +106,7 @@ set(MAX_RX_PDO "4"
set(MAX_ERRORS "4"
CACHE STRING "max size of error list")

set(SDO_TIMEOUT "100"
set(SDO_TIMEOUT "200"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This changes the default timeout for every user of c-open. Note that this is an option that can be adjusted locally if a particular application has different requirements.

CACHE STRING "timeout in ms for ongoing SDO transfers")

set(CO_THREAD_PRIO "10"
16 changes: 12 additions & 4 deletions include/co_api.h
View file
Original file line number Diff line number Diff line change
@@ -223,11 +223,19 @@ typedef enum co_dtype
/** Access function event */
typedef enum od_event
{
OD_EVENT_READ, /**< Read subindex */
OD_EVENT_WRITE, /**< Write subindex */
OD_EVENT_RESTORE, /**< Restore default value */
OD_EVENT_READ, /**< Read subindex */
OD_EVENT_WRITE, /**< Write subindex */
OD_EVENT_RESTORE, /**< Restore default value */
} od_event_t;

/** Notify event */
typedef enum od_notify_event
{
OD_NOTIFY_ACCESSED,
OD_NOTIFY_VALUE_SET,
OD_NOTIFY_SDO_RECEIVED,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are both ACCESSED and VALUE_SET required? They are both called from co_od_set_value and it seems that the only difference is whether the user has specified that the object has an accessor function or not.

SDO_RECEIVED seems to be used when an object larger than 64 bits is being downloaded and will be called for every partial update with the size received so far. So the value in this case is a size. This should be documented. There should also be a test that verifies the functionality.

} od_notify_event_t;

struct co_obj;
struct co_entry;

@@ -317,7 +325,7 @@ typedef struct co_cfg
uint8_t msef[5]);

/** Notify callback */
void (*cb_notify) (co_net_t * net, uint16_t index, uint8_t subindex);
void (*cb_notify) (co_net_t * net, uint16_t index, uint8_t subindex, od_notify_event_t event, uint32_t value);

/** Function to open dictionary store */
void * (*open) (co_store_t store, co_mode_t mode);
2 changes: 2 additions & 0 deletions src/co_main.c
View file
Original file line number Diff line number Diff line change
@@ -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;
@@ -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;
2 changes: 1 addition & 1 deletion src/co_main.h
View file
Original file line number Diff line number Diff line change
@@ -277,7 +277,7 @@ struct co_net
uint8_t msef[5]);

/** Notify callback */
void (*cb_notify) (co_net_t * net, uint16_t index, uint8_t subindex);
void (*cb_notify) (co_net_t * net, uint16_t index, uint8_t subindex, od_notify_event_t event, uint32_t value);

/** Function to open dictionary store */
void * (*open) (co_store_t store, co_mode_t mode);
12 changes: 7 additions & 5 deletions src/co_od.c
View file
Original file line number Diff line number Diff line change
@@ -36,16 +36,18 @@ 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,
uint8_t subindex)
uint8_t subindex,
od_notify_event_t event,
uint32_t value)
{
if (entry->flags & OD_NOTIFY)
{
if (net->cb_notify)
net->cb_notify (net, obj->index, subindex);
net->cb_notify (net, obj->index, subindex, event, value);
}
}

@@ -221,7 +223,7 @@ uint32_t co_od_set_value (
uint32_t v = value;

result = obj->access (net, OD_EVENT_WRITE, obj, entry, subindex, &v);
co_od_notify (net, obj, entry, subindex);
co_od_notify (net, obj, entry, subindex, OD_NOTIFY_ACCESSED, 0);
return result;
}

@@ -262,7 +264,7 @@ uint32_t co_od_set_value (
return CO_SDO_ABORT_GENERAL;
}

co_od_notify (net, obj, entry, subindex);
co_od_notify (net, obj, entry, subindex, OD_NOTIFY_VALUE_SET, 0);
return 0;
}

21 changes: 21 additions & 0 deletions src/co_od.h
View file
Original file line number Diff line number Diff line change
@@ -271,6 +271,27 @@ 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
* @param event event type
* @param value optional value
*/
void co_od_notify (
co_net_t * net,
const co_obj_t * obj,
const co_entry_t * entry,
uint8_t subindex,
od_notify_event_t event,
uint32_t value);

#ifdef __cplusplus
}
#endif
11 changes: 6 additions & 5 deletions src/co_sdo_client.c
View file
Original file line number Diff line number Diff line change
@@ -154,15 +154,15 @@ static int co_sdo_tx_download_init_rsp (
memcpy (&msg[1], job->sdo.data, size);

msg[0] = CO_SDO_CCS_DOWNLOAD_SEG_REQ | ((7 - (size & 0x07)) << 1);
if (size < 7)
msg[0] |= CO_SDO_C;

job->sdo.toggle = 0;

job->sdo.data += size;
job->sdo.remain -= size;
job->sdo.total += size;

if (job->sdo.remain == 0)
msg[0] |= CO_SDO_C;

os_channel_send (net->channel, 0x600 + node, msg, sizeof (msg));
}

@@ -209,13 +209,14 @@ static int co_sdo_tx_download_seg_rsp (
msg[0] = CO_SDO_CCS_DOWNLOAD_SEG_REQ | ((7 - (size & 0x07)) << 1);
if (job->sdo.toggle)
msg[0] |= CO_SDO_TOGGLE;
if (size < 7)
msg[0] |= CO_SDO_C;

job->sdo.data += size;
job->sdo.remain -= size;
job->sdo.total += size;

if (job->sdo.remain == 0)
msg[0] |= CO_SDO_C;

os_channel_send (net->channel, 0x600 + node, msg, sizeof (msg));
}

226 changes: 144 additions & 82 deletions src/co_sdo_server.c
View file
Original file line number Diff line number Diff line change
@@ -28,27 +28,6 @@

#include <inttypes.h>

#define CO_SDO_xCS(v) ((v)&0xE0)
#define CO_SDO_N(v) (((v) >> 2) & 0x03)
#define CO_SDO_E BIT (1)
#define CO_SDO_S BIT (0)

#define CO_SDO_CCS_DOWNLOAD_SEG_REQ (0 << 5)
#define CO_SDO_CCS_DOWNLOAD_INIT_REQ (1 << 5)
#define CO_SDO_CCS_UPLOAD_INIT_REQ (2 << 5)
#define CO_SDO_CCS_UPLOAD_SEG_REQ (3 << 5)

#define CO_SDO_SCS_UPLOAD_SEG_RSP (0 << 5)
#define CO_SDO_SCS_DOWNLOAD_SEG_RSP (1 << 5)
#define CO_SDO_SCS_UPLOAD_INIT_RSP (2 << 5)
#define CO_SDO_SCS_DOWNLOAD_INIT_RSP (3 << 5)

#define CO_SDO_xCS_ABORT (4 << 5)

#define CO_SDO_TOGGLE BIT (4)
#define CO_SDO_N_SEG(v) (((v) >> 1) & 0x07)
#define CO_SDO_C BIT (0)

void co_sdo_abort (
co_net_t * net,
uint16_t id,
@@ -129,6 +108,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 */
@@ -182,8 +162,7 @@ static int co_sdo_rx_upload_init_req (
if (job->sdo.remain <= sizeof (job->sdo.value))
{
/* Object values up to 64 bits are fetched atomically */
abort =
co_od_get_value (net, obj, entry, job->sdo.subindex, &job->sdo.value);
abort = co_od_get_value (net, obj, entry, job->sdo.subindex, &job->sdo.value);
job->sdo.data = (uint8_t *)&job->sdo.value;
}
else
@@ -289,6 +268,48 @@ static int co_sdo_rx_upload_seg_req (
return 0;
}

static bool co_is_datatype_atomic (co_dtype_t datatype)
{
switch (datatype)
{
case DTYPE_BOOLEAN:
case DTYPE_INTEGER8:
case DTYPE_INTEGER16:
case DTYPE_INTEGER32:
case DTYPE_UNSIGNED8:
case DTYPE_UNSIGNED16:
case DTYPE_UNSIGNED32:
case DTYPE_REAL32:
case DTYPE_REAL64:
case DTYPE_INTEGER64:
case DTYPE_UNSIGNED64:
return true;

case DTYPE_VISIBLE_STRING:
case DTYPE_OCTET_STRING:
case DTYPE_UNICODE_STRING:
case DTYPE_TIME_OF_DAY:
case DTYPE_TIME_DIFFERENCE:
case DTYPE_DOMAIN:
case DTYPE_INTEGER24:
case DTYPE_INTEGER40:
case DTYPE_INTEGER48:
case DTYPE_INTEGER56:
case DTYPE_UNSIGNED24:
case DTYPE_UNSIGNED40:
case DTYPE_UNSIGNED48:
case DTYPE_UNSIGNED56:
case DTYPE_PDO_COMM_PARAM:
case DTYPE_PDO_MAPPING:
case DTYPE_SDO_PARAM:
case DTYPE_IDENTITY:
return false;

default:
return false;
}
}

static int co_sdo_rx_download_init_req (
co_net_t * net,
uint8_t node,
@@ -306,6 +327,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 */
@@ -346,40 +368,13 @@ static int co_sdo_rx_download_init_req (
return -1;
}

job->sdo.remain = CO_BYTELENGTH (entry->bitlength);
job->sdo.toggle = 0;

if (job->sdo.remain <= sizeof (job->sdo.value))
{
/* Object values up to 64 bits are cached so that we can set
them atomically when the transfer is complete */
job->sdo.data = (uint8_t *)&job->sdo.value;
job->sdo.cached = true;
}
else
{
/* Otherwise a pointer is used to access object */
abort = co_od_get_ptr (net, obj, entry, job->sdo.subindex, &job->sdo.data);
if (abort)
{
co_sdo_abort (
net,
0x580 + net->node,
job->sdo.index,
job->sdo.subindex,
abort);
return -1;
}
}

/* Check for expedited download */
if (type & CO_SDO_E)
{
size_t size = (type & CO_SDO_S) ? 4 - CO_SDO_N (type) : 4;
uint32_t value;

/* Validate size */
if (size != job->sdo.remain)
if (size > CO_BYTELENGTH (entry->bitlength))
{
co_sdo_abort (
net,
@@ -390,11 +385,35 @@ static int co_sdo_rx_download_init_req (
return -1;
}

/* Fetch value */
value = co_fetch_uint32 (&data[4]);
if (co_is_datatype_atomic (entry->datatype))
{
uint32_t value;

/* Fetch value */
value = co_fetch_uint32 (&data[4]);

/* Atomically set value */
abort = co_od_set_value (net, obj, entry, job->sdo.subindex, value);
}
else
{
/* Pointer is used to access object */
abort = co_od_get_ptr (net, obj, entry, job->sdo.subindex, &job->sdo.data);
if (abort)
{
co_sdo_abort (
net,
0x580 + net->node,
job->sdo.index,
job->sdo.subindex,
abort);
return -1;
}

memcpy (job->sdo.data, &data[4], size);

/* Atomically set value */
abort = co_od_set_value (net, obj, entry, job->sdo.subindex, value);
co_od_notify (net, obj, entry, job->sdo.subindex, OD_NOTIFY_SDO_RECEIVED, size);
}

/* Done */
job->type = CO_JOB_NONE;
@@ -410,6 +429,46 @@ static int co_sdo_rx_download_init_req (
return -1;
}
}
else
{
job->sdo.total = co_fetch_uint32 (&data[4]);
job->sdo.remain = job->sdo.total;

if (job->sdo.remain > CO_BYTELENGTH (entry->bitlength))
{
co_sdo_abort (
net,
0x580 + net->node,
job->sdo.index,
job->sdo.subindex,
CO_SDO_ABORT_LENGTH);
return -1;
}
job->sdo.toggle = 0;

if (co_is_datatype_atomic (entry->datatype))
{
/* Datatypes with atomic functions are cached in job->sdo.value
* so they can be set atomically when the transfer is complete */
job->sdo.data = (uint8_t *)&job->sdo.value;
job->sdo.cached = true;
}
else
{
/* Otherwise a pointer is used to access object */
abort = co_od_get_ptr (net, obj, entry, job->sdo.subindex, &job->sdo.data);
if (abort)
{
co_sdo_abort (
net,
0x580 + net->node,
job->sdo.index,
job->sdo.subindex,
abort);
return -1;
}
}
}

/* Dictionary has been written to and is now dirty */
net->config_dirty = 1;
@@ -466,37 +525,36 @@ 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);
abort = co_od_set_value (net, obj, entry, job->sdo.subindex, job->sdo.value);
if (abort)
{
co_sdo_abort (
@@ -508,6 +566,10 @@ static int co_sdo_rx_download_seg_req (
return -1;
}
}
else
{
co_od_notify (net, obj, entry, job->sdo.subindex, OD_NOTIFY_SDO_RECEIVED, job->sdo.total);
}
}

/* Segmented response */
2 changes: 1 addition & 1 deletion test/mocks.cpp
View file
Original file line number Diff line number Diff line change
@@ -175,7 +175,7 @@ void cb_sync (co_net_t * net)
unsigned int cb_notify_calls;
uint16_t cb_notify_index;
uint16_t cb_notify_subindex;
void cb_notify (co_net_t * net, uint16_t index, uint8_t subindex)
void cb_notify (co_net_t * net, uint16_t index, uint8_t subindex, od_notify_event_t event, uint32_t value)
{
cb_notify_calls++;
cb_notify_index = index;
2 changes: 1 addition & 1 deletion test/mocks.h
View file
Original file line number Diff line number Diff line change
@@ -108,7 +108,7 @@ void cb_sync (co_net_t * net);
extern unsigned int cb_notify_calls;
extern uint16_t cb_notify_index;
extern uint16_t cb_notify_subindex;
void cb_notify (co_net_t * net, uint16_t index, uint8_t subindex);
void cb_notify (co_net_t * net, uint16_t index, uint8_t subindex, od_notify_event_t event, uint32_t value);

void store_init (void);
extern unsigned int store_open_calls;
4 changes: 2 additions & 2 deletions test/test_sdo_client.cpp
View file
Original file line number Diff line number Diff line change
@@ -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] = {
@@ -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] = {
1 change: 1 addition & 0 deletions test/test_sdo_server.cpp
View file
Original file line number Diff line number Diff line change
@@ -132,6 +132,7 @@ TEST_F (SdoServerTest, SegmentedDownload)
}

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

TEST_F (SdoServerTest, SegmentedDownloadCached)
2 changes: 1 addition & 1 deletion test/test_util.h
View file
Original file line number Diff line number Diff line change
@@ -95,7 +95,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];
2 changes: 1 addition & 1 deletion util/slave/slave.c
View file
Original file line number Diff line number Diff line change
@@ -201,7 +201,7 @@ static void cb_sync (co_net_t * net)
}

/* Called when RPDO is received (if OD_NOTIFY is set) */
static void cb_notify (co_net_t * net, uint16_t index, uint8_t subindex)
static void cb_notify (co_net_t * net, uint16_t index, uint8_t subindex, od_notify_event_t event, uint32_t value)
{
}