Skip to content

New CDD API #172

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

Merged
merged 1 commit into from
Feb 13, 2025
Merged

New CDD API #172

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
66 changes: 66 additions & 0 deletions gateway_to_backend/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -277,6 +277,56 @@ _Even if this version is increased, the API remains backward compatible.
It just help backends development to identify if new features are
present in a gateway._

#### Set configuration data item

Only processed if CONFIGURATION_DATA_V1 feature flag is set on the gateway.

- **Request:**

> **topic:** gw-request/set_configuration_data_item/*\<gw-id\>/\<sink-id\>*
>
> **content:** [GenericMessage][message_GenericMessage].[WirepasMessage][message_WirepasMessage].SetConfigurationDataItemReq

- **Response:**

> **topics:** gw-response/set_configuration_data_item/*\<gw-id\>/\<sink-id\>*
>
> **content:** [GenericMessage][message_GenericMessage].[WirepasMessage][message_WirepasMessage].SetConfigurationDataItemResp
>
> **possible errors:**
> - INVALID_ROLE: Node is not a sink.
> - SINK_OUT_OF_MEMORY: Payload is too large or maximum number of items is reached.
> - INVALID_DEST_ENDPOINT: Endpoint is too large (it should be a 16 bit unsigned integer).
> - INVALID_DATA_PAYLOAD: Payload is rejected for this endpoint by the sink.
> - INVALID_SINK_ID: Invalid sink id in the request header.

:warning:

_Successfully setting an item must be treated as a configuration change on the
sink, meaning that the gateway must report the new complete configuration data
content by generating a new [StatusEvent][message_StatusEvent] message._

#### Get configuration data item

Only processed if CONFIGURATION_DATA_V1 feature flag is set on the gateway.

- **Request:**

> **topic:** gw-request/get_configuration_data_item/*\<gw-id\>/\<sink-id\>*
>
> **content:** [GenericMessage][message_GenericMessage].[WirepasMessage][message_WirepasMessage].GetConfigurationDataItemReq

- **Response:**

> **topics:** gw-response/get_configuration_data_item/*\<gw-id\>/\<sink-id\>*
>
> **content:** [GenericMessage][message_GenericMessage].[WirepasMessage][message_WirepasMessage].GetConfigurationDataItemResp
>
> **possible errors:**
> - INVALID_DEST_ENDPOINT: Endpoint is too large (it should be a 16 bit unsigned integer).
> - INVALID_PARAM: No item found with the given endpoint.
> - INVALID_SINK_ID: Invalid sink id in the request header.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I suppose having nodes with non-sink role connected as sinks to the gateway is not a supported scenario, so no need to define what happens if the CDC contents change on such a system?

Copy link
Contributor

Choose a reason for hiding this comment

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

I guess not for that specific case. However, I do think that either the naming of the topic or alternative this documentation would need to be tuned somehow to explain what does these actually configure. Now the topic name is very close to set_config, so it kind of makes you to assume that you are setting part of that config.

This is anyways mesh level CDD content item, so I wonder should these be set_mesh_config_item or set_mesh_config_data_item. Or if we want to be more explicit, could be set_cdc_item, which would not have possibility to misunderstanding, but would require explanation what it means.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah this is a tricky one, we decided to use naming that is consistent with DECT-2020 specification (page 59), but I agree on it's own the naming is not very self explanatory in this interface.

### Data module

---
Expand Down Expand Up @@ -518,6 +568,9 @@ definition)

gw-request/otap_set_target_scratchpad/<gw-id>/<sink-id>

gw-request/set_configuration_data_item/<gw-id>/<sink-id>
Copy link
Contributor

Choose a reason for hiding this comment

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

Now thinking of it, I do think that we should probably have similar get API for this. Then it would be more analogical with other setter type of API methods (you can read all of those with corresponding get method). While that is to some extent available at the moment as well, there is just extra overhead if we would need to query whole sink configs if some backend component is just interested on its own appconfig type of application configuration etc.


gw-request/get_configuration_data_item/<gw-id>/<sink-id>
```

*Response* from a gateway to a backend:
Expand All @@ -538,6 +591,10 @@ definition)
gw-response/otap_process_scratchpad/<gw-id>/<sink-id>

gw-response/otap_set_target_scratchpad/<gw-id>/<sink-id>

gw-response/set_configuration_data_item/<gw-id>/<sink-id>

gw-response/get_configuration_data_item/<gw-id>/<sink-id>
```

*Asynchronous* event from a gateway:
Expand Down Expand Up @@ -591,4 +648,13 @@ definition)

[message_SetScratchpadTargetAndActionResp]: https://github.com/wirepas/backend-apis/blob/77d83cc91ccfede5e457a488f0611f1fed342585/gateway_to_backend/protocol_buffers_files/otap_message.proto#L54-L56

<!-- TODO update links and take them into use once they are available in the master branch -->
<!-- [message_SetConfigurationDataItemReq]: -->
<!---->
<!-- [message_SetConfigurationDataItemResp]: -->
<!---->
<!-- [message_GetConfigurationDataItemReq]: -->
<!---->
<!-- [message_GetConfigurationDataItemResp]: -->

[protobuf_homepage]: https://developers.google.com/protocol-buffers
35 changes: 35 additions & 0 deletions gateway_to_backend/protocol_buffers_files/config_message.proto
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,11 @@ message NetworkKeys {
required bytes authentication = 2;
}

message ConfigurationDataItem {
required uint32 endpoint = 1;
required bytes payload = 2;
Copy link
Contributor

Choose a reason for hiding this comment

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

Should the payload length be given as parameter here as well? Or is that something that we can expect to be achievable (in all the relevant client libraries / languages) via the payload itself?

Copy link
Contributor Author

@sgunes-wirepas sgunes-wirepas Jan 14, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

bytes proto type holds the length already, it should be possible to obtain it from the libraries

}

message SinkReadConfig {
// Local id to uniquely identify a sink on gateway
required string sink_id = 1;
Expand Down Expand Up @@ -72,6 +77,9 @@ message SinkReadConfig {
optional ScratchpadInfo processed_scratchpad = 21;
optional uint32 firmware_area_id = 22;
optional TargetScratchpadAndAction target_and_action = 23; // Unset if sink doesn't support it

// Only present if CDD_API feature flag is set
repeated ConfigurationDataItem configuration_data_content = 24;
Copy link
Contributor

Choose a reason for hiding this comment

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

Hmm, what is included in this?

Based on the initial discussion, I thought that we agreed that this kind of "listing" all is a bit questionable. There might be different backends managing different items in the overall CDD list. Should we allow all backends to read all, or should be per request (i.e. one can ask for the endpoints of interest and receives only those).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

All of the items would be included here, and there is no possibility of requesting to read a single item.

This means all of the information related to CDC would be already available in the status event messages. Maybe @GwendalRaoul can tell more about the reasoning behind having only a list based on his experience?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeap, I was mainly raising the point that this is not exactly according to the (initial) discussion. In case of CDD there will be multiple masters for the managed data and I would think that it would make sense to try to not mix those. If we want to have it in status message, then I guess everything needs to be listed there.

I'm not against lists as such, those would be beneficial in any case (meaning also with the "list of endpoints" case), but just wanted to raise the concern on "coupling" all the CDD items.

}

message SinkNewConfig {
Expand Down Expand Up @@ -118,6 +126,8 @@ message GatewayInfo {
// it must be sent as chunk. If not set, it means that
// scratchpad can be sent in a single request
optional uint32 max_scratchpad_size = 5;

repeated GatewayFeature gw_features = 6;
}

/*
Expand Down Expand Up @@ -146,6 +156,8 @@ message GatewayInfo {

// See GatewayInfo for details of this field
optional uint32 max_scratchpad_size = 7;

repeated GatewayFeature gw_features = 8;
}

message GetConfigsReq {
Expand Down Expand Up @@ -180,3 +192,26 @@ message GetGwInfoResp {

required GatewayInfo info = 2;
}

message SetConfigurationDataItemReq {
required RequestHeader header = 1;

// 0-length payloads are used to clear an entry
required ConfigurationDataItem configuration_data_item = 2;
Copy link
Contributor

Choose a reason for hiding this comment

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

I wonder whether this should be repeated list still? That would at least then leave the decision whether to update one or more at a time to the client side.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If it's repeated, I think we would need to send all of the CDC items in the reply. Since they would be set one by one on the sink, some of the sets might be successful and some not.

Copy link
Contributor

Choose a reason for hiding this comment

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

Right indeed, this is tricky part as each item might be handled in the end by different entity in the sink and naturally there is no "transactionality" with these. I guess it is not too big of a drawback, but maybe something to worth discussing with wider audience as well next week.

}

message SetConfigurationDataItemResp {
required ResponseHeader header = 1;
}

message GetConfigurationDataItemReq {
required RequestHeader header = 1;

required uint32 endpoint = 2;
}

message GetConfigurationDataItemResp {
required ResponseHeader header = 1;

optional ConfigurationDataItem configuration_data_item = 2;
}
6 changes: 5 additions & 1 deletion gateway_to_backend/protocol_buffers_files/generic_message.proto
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,10 @@ message WirepasMessage {
optional GetGwInfoResp get_gateway_info_resp = 16;
optional SetScratchpadTargetAndActionReq set_scratchpad_target_and_action_req = 17;
optional SetScratchpadTargetAndActionResp set_scratchpad_target_and_action_resp = 18;
optional SetConfigurationDataItemReq set_configuration_data_item_req = 19;
optional SetConfigurationDataItemResp set_configuration_data_item_resp = 20;
optional GetConfigurationDataItemReq get_configuration_data_item_req = 21;
optional GetConfigurationDataItemResp get_configuration_data_item_resp = 22;
}

message CustomerMessage {
Expand All @@ -36,4 +40,4 @@ message CustomerMessage {
message GenericMessage {
optional WirepasMessage wirepas = 1;
optional CustomerMessage customer = 2;
}
}
12 changes: 11 additions & 1 deletion gateway_to_backend/protocol_buffers_files/wp_global.proto
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -103,4 +103,14 @@ message TargetScratchpadAndAction {
ProcessingDelay delay = 4; // Delay parameter for action PROPAGATE_AND_PROCESS_WITH_DELAY
uint32 raw = 5; // Raw parameter for the action (between 0 and 255)
}
}
}

enum GatewayFeature {
UNKNOWN = 0;
// Support for uploading a scratchpad in chunks
SCRATCHPAD_CHUNK_V1 = 1;
// Configuration Data Distribution API, allows modifying of the
// configuration data content.
CONFIGURATION_DATA_V1 = 2;
}