diff mbox series

[RFC,v1,1/4] suricatta: add basic status request

Message ID 20210924092411.10768-2-roland.gaudig-oss@weidmueller.com
State Changes Requested
Headers show
Series suricatta: ipc: add request to get hawkbit server status | expand

Commit Message

Roland Gaudig Sept. 24, 2021, 9:24 a.m. UTC 
From: Roland Gaudig <roland.gaudig@weidmueller.com>

Signed-off-by: Roland Gaudig <roland.gaudig@weidmueller.com>
---

 include/network_ipc.h      |  3 ++-
 suricatta/server_hawkbit.c | 22 +++++++++++++++++++++-
 suricatta/server_hawkbit.h |  2 ++
 3 files changed, 25 insertions(+), 2 deletions(-)

Comments

Stefano Babic Oct. 13, 2021, 12:10 p.m. UTC | #1 
Hi Roland,

On 24.09.21 11:24, Roland Gaudig wrote:
> From: Roland Gaudig <roland.gaudig@weidmueller.com>
> 
> Signed-off-by: Roland Gaudig <roland.gaudig@weidmueller.com>
> ---
> 
>   include/network_ipc.h      |  3 ++-
>   suricatta/server_hawkbit.c | 22 +++++++++++++++++++++-
>   suricatta/server_hawkbit.h |  2 ++
>   3 files changed, 25 insertions(+), 2 deletions(-)
> 
> diff --git a/include/network_ipc.h b/include/network_ipc.h
> index 1a3d957..84cd7b1 100644
> --- a/include/network_ipc.h
> +++ b/include/network_ipc.h
> @@ -47,7 +47,8 @@ typedef enum {
>   enum {
>   	CMD_ACTIVATION,	/* this returns the answer if a SW can be activated */
>   	CMD_CONFIG,
> -	CMD_ENABLE	/* Enable or disable suricatta mode */
> +	CMD_ENABLE,	/* Enable or disable suricatta mode */
> +	CMD_GET_STATUS
>   };
>   
>   enum run_type {
> diff --git a/suricatta/server_hawkbit.c b/suricatta/server_hawkbit.c
> index 805158d..2298119 100644
> --- a/suricatta/server_hawkbit.c
> +++ b/suricatta/server_hawkbit.c
> @@ -543,7 +543,11 @@ static server_op_res_t server_get_device_info(channel_t *channel, channel_data_t
>   		result = SERVER_EINIT;
>   		goto cleanup;
>   	}
> -	if ((result = map_channel_retcode(channel->get(channel, (void *)channel_data))) !=
> +
> +	channel_op_res_t ch_response = channel->get(channel, (void *)channel_data);
> +	server_hawkbit.server_status = ch_response;
> +	server_hawkbit.server_status_time = time(NULL);

I just ask about the format of date/time to be reported to the caller. 
This just returns seconds from EPOCH. Is it this a suitable format to 
deliver date / time ?

SWUpdate has already support for ISO_8601. Function 
swupdate_time_iso8601() returns the string from gettimeofday, but it can 
be changed to accept a parameter.


> +	if ((result = map_channel_retcode(ch_response)) !=
>   	    SERVER_OK) {
>   		goto cleanup;
>   	}
> @@ -2026,6 +2030,19 @@ static server_op_res_t server_configuration_ipc(ipc_message *msg)
>   	return SERVER_OK;
>   }
>   
> +static server_op_res_t server_status_ipc(ipc_message *msg)
> +{
> +	TRACE("Received get status\n%s\n", msg->data.procmsg.buf);  // TODO clean-up
> +
> +	sprintf(msg->data.procmsg.buf,
> +		"{\"server\":{\"status\":%d,\"time\":%ld}}",
> +		server_hawkbit.server_status,
> +		server_hawkbit.server_status_time);
> +	msg->data.procmsg.len = strlen(msg->data.procmsg.buf);
> +
> +	return SERVER_OK;
> +}
> +
>   server_op_res_t server_ipc(ipc_message *msg)
>   {
>   	server_op_res_t result = SERVER_OK;
> @@ -2037,6 +2054,9 @@ server_op_res_t server_ipc(ipc_message *msg)
>   	case CMD_CONFIG:
>   		result = server_configuration_ipc(msg);
>   		break;
> +	case CMD_GET_STATUS:
> +		result = server_status_ipc(msg);
> +		break;
>   	default:
>   		result = SERVER_EERR;
>   		break;
> diff --git a/suricatta/server_hawkbit.h b/suricatta/server_hawkbit.h
> index aea4bb0..3b155f3 100644
> --- a/suricatta/server_hawkbit.h
> +++ b/suricatta/server_hawkbit.h
> @@ -44,6 +44,8 @@ typedef struct {
>   	char *cached_file;
>   	bool usetokentodwl;
>   	unsigned int initial_report_resend_period;
> +	int server_status;
> +	time_t server_status_time;
>   } server_hawkbit_t;
>   
>   extern server_hawkbit_t server_hawkbit;
> 

Best regards,
Stefano Babic
Roland Gaudig Oct. 13, 2021, 12:38 p.m. UTC | #2 
On 13.10.21 12:10, Stefano Babic wrote:
> Hi Roland,
> 
> On 24.09.21 11:24, Roland Gaudig wrote:
>> From: Roland Gaudig <roland.gaudig@weidmueller.com>
>>
>> Signed-off-by: Roland Gaudig <roland.gaudig@weidmueller.com>
>> ---
>>
>>   include/network_ipc.h      |  3 ++-
>>   suricatta/server_hawkbit.c | 22 +++++++++++++++++++++-
>>   suricatta/server_hawkbit.h |  2 ++
>>   3 files changed, 25 insertions(+), 2 deletions(-)
>>
>> diff --git a/include/network_ipc.h b/include/network_ipc.h
>> index 1a3d957..84cd7b1 100644
>> --- a/include/network_ipc.h
>> +++ b/include/network_ipc.h
>> @@ -47,7 +47,8 @@ typedef enum {
>>   enum {
>>       CMD_ACTIVATION,    /* this returns the answer if a SW can be
>> activated */
>>       CMD_CONFIG,
>> -    CMD_ENABLE    /* Enable or disable suricatta mode */
>> +    CMD_ENABLE,    /* Enable or disable suricatta mode */
>> +    CMD_GET_STATUS
>>   };
>>     enum run_type {
>> diff --git a/suricatta/server_hawkbit.c b/suricatta/server_hawkbit.c
>> index 805158d..2298119 100644
>> --- a/suricatta/server_hawkbit.c
>> +++ b/suricatta/server_hawkbit.c
>> @@ -543,7 +543,11 @@ static server_op_res_t
>> server_get_device_info(channel_t *channel, channel_data_t
>>           result = SERVER_EINIT;
>>           goto cleanup;
>>       }
>> -    if ((result = map_channel_retcode(channel->get(channel, (void
>> *)channel_data))) !=
>> +
>> +    channel_op_res_t ch_response = channel->get(channel, (void
>> *)channel_data);
>> +    server_hawkbit.server_status = ch_response;
>> +    server_hawkbit.server_status_time = time(NULL);
> 
> I just ask about the format of date/time to be reported to the caller.
> This just returns seconds from EPOCH. Is it this a suitable format to
> deliver date / time ?
>

I was unsure about the time format. For testing purposes I would have
preferred a more human readable format like swupdate_time_iso8601()
delivers. On the other hand the colleague who uses the other side of the
interface asked me for a value in seconds from EPOCH. But I think it
would not be too hard to change this.

I would go with either way. Which format would be the more useful for
most use cases?

> SWUpdate has already support for ISO_8601. Function
> swupdate_time_iso8601() returns the string from gettimeofday, but it can
> be changed to accept a parameter.

I think the time value from gettimeofday would be OK for our needs and
the string returned by swupdate_time_iso8601() looks good. I think there
is no need for a parameter.

> 
>> +    if ((result = map_channel_retcode(ch_response)) !=
>>           SERVER_OK) {
>>           goto cleanup;
>>       }
>> @@ -2026,6 +2030,19 @@ static server_op_res_t
>> server_configuration_ipc(ipc_message *msg)
>>       return SERVER_OK;
>>   }
>>   +static server_op_res_t server_status_ipc(ipc_message *msg)
>> +{
>> +    TRACE("Received get status\n%s\n", msg->data.procmsg.buf);  //
>> TODO clean-up
>> +
>> +    sprintf(msg->data.procmsg.buf,
>> +        "{\"server\":{\"status\":%d,\"time\":%ld}}",
>> +        server_hawkbit.server_status,
>> +        server_hawkbit.server_status_time);
>> +    msg->data.procmsg.len = strlen(msg->data.procmsg.buf);
>> +
>> +    return SERVER_OK;
>> +}
>> +
>>   server_op_res_t server_ipc(ipc_message *msg)
>>   {
>>       server_op_res_t result = SERVER_OK;
>> @@ -2037,6 +2054,9 @@ server_op_res_t server_ipc(ipc_message *msg)
>>       case CMD_CONFIG:
>>           result = server_configuration_ipc(msg);
>>           break;
>> +    case CMD_GET_STATUS:
>> +        result = server_status_ipc(msg);
>> +        break;
>>       default:
>>           result = SERVER_EERR;
>>           break;
>> diff --git a/suricatta/server_hawkbit.h b/suricatta/server_hawkbit.h
>> index aea4bb0..3b155f3 100644
>> --- a/suricatta/server_hawkbit.h
>> +++ b/suricatta/server_hawkbit.h
>> @@ -44,6 +44,8 @@ typedef struct {
>>       char *cached_file;
>>       bool usetokentodwl;
>>       unsigned int initial_report_resend_period;
>> +    int server_status;
>> +    time_t server_status_time;
>>   } server_hawkbit_t;
>>     extern server_hawkbit_t server_hawkbit;
>>
> 

Best regards,
Roland Gaudig
Stefano Babic Oct. 13, 2021, 1 p.m. UTC | #3 
Hi Roland,

On 13.10.21 14:38, Roland Gaudig wrote:
> On 13.10.21 12:10, Stefano Babic wrote:
>> Hi Roland,
>>
>> On 24.09.21 11:24, Roland Gaudig wrote:
>>> From: Roland Gaudig <roland.gaudig@weidmueller.com>
>>>
>>> Signed-off-by: Roland Gaudig <roland.gaudig@weidmueller.com>
>>> ---
>>>
>>>    include/network_ipc.h      |  3 ++-
>>>    suricatta/server_hawkbit.c | 22 +++++++++++++++++++++-
>>>    suricatta/server_hawkbit.h |  2 ++
>>>    3 files changed, 25 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/include/network_ipc.h b/include/network_ipc.h
>>> index 1a3d957..84cd7b1 100644
>>> --- a/include/network_ipc.h
>>> +++ b/include/network_ipc.h
>>> @@ -47,7 +47,8 @@ typedef enum {
>>>    enum {
>>>        CMD_ACTIVATION,    /* this returns the answer if a SW can be
>>> activated */
>>>        CMD_CONFIG,
>>> -    CMD_ENABLE    /* Enable or disable suricatta mode */
>>> +    CMD_ENABLE,    /* Enable or disable suricatta mode */
>>> +    CMD_GET_STATUS
>>>    };
>>>      enum run_type {
>>> diff --git a/suricatta/server_hawkbit.c b/suricatta/server_hawkbit.c
>>> index 805158d..2298119 100644
>>> --- a/suricatta/server_hawkbit.c
>>> +++ b/suricatta/server_hawkbit.c
>>> @@ -543,7 +543,11 @@ static server_op_res_t
>>> server_get_device_info(channel_t *channel, channel_data_t
>>>            result = SERVER_EINIT;
>>>            goto cleanup;
>>>        }
>>> -    if ((result = map_channel_retcode(channel->get(channel, (void
>>> *)channel_data))) !=
>>> +
>>> +    channel_op_res_t ch_response = channel->get(channel, (void
>>> *)channel_data);
>>> +    server_hawkbit.server_status = ch_response;
>>> +    server_hawkbit.server_status_time = time(NULL);
>>
>> I just ask about the format of date/time to be reported to the caller.
>> This just returns seconds from EPOCH. Is it this a suitable format to
>> deliver date / time ?
>>
> 
> I was unsure about the time format. For testing purposes I would have
> preferred a more human readable format like swupdate_time_iso8601()
> delivers. On the other hand the colleague who uses the other side of the
> interface asked me for a value in seconds from EPOCH. But I think it
> would not be too hard to change this.

Ok

> 
> I would go with either way. Which format would be the more useful for
> most use cases?

If I simply search for "standard" date / time format, I hit again 
ISO8601. And this is the only format I was asked up now.

> 
>> SWUpdate has already support for ISO_8601. Function
>> swupdate_time_iso8601() returns the string from gettimeofday, but it can
>> be changed to accept a parameter.
> 
> I think the time value from gettimeofday would be OK for our needs and
> the string returned by swupdate_time_iso8601() looks good. I think there
> is no need for a parameter.

Ok - the only difference is that the reported time is then the time by 
sending the IPC command, and not the time when SWUpdate asked for status 
(last polling). If you need the last one, you can just store a timeval 
instead of time_t returned by gettimeofday() and adjust 
swupdate_time_iso8601() to get a parameter.

> 
>>
>>> +    if ((result = map_channel_retcode(ch_response)) !=
>>>            SERVER_OK) {
>>>            goto cleanup;
>>>        }
>>> @@ -2026,6 +2030,19 @@ static server_op_res_t
>>> server_configuration_ipc(ipc_message *msg)
>>>        return SERVER_OK;
>>>    }
>>>    +static server_op_res_t server_status_ipc(ipc_message *msg)
>>> +{
>>> +    TRACE("Received get status\n%s\n", msg->data.procmsg.buf);  //
>>> TODO clean-up
>>> +
>>> +    sprintf(msg->data.procmsg.buf,
>>> +        "{\"server\":{\"status\":%d,\"time\":%ld}}",
>>> +        server_hawkbit.server_status,
>>> +        server_hawkbit.server_status_time);
>>> +    msg->data.procmsg.len = strlen(msg->data.procmsg.buf);
>>> +
>>> +    return SERVER_OK;
>>> +}
>>> +
>>>    server_op_res_t server_ipc(ipc_message *msg)
>>>    {
>>>        server_op_res_t result = SERVER_OK;
>>> @@ -2037,6 +2054,9 @@ server_op_res_t server_ipc(ipc_message *msg)
>>>        case CMD_CONFIG:
>>>            result = server_configuration_ipc(msg);
>>>            break;
>>> +    case CMD_GET_STATUS:
>>> +        result = server_status_ipc(msg);
>>> +        break;
>>>        default:
>>>            result = SERVER_EERR;
>>>            break;
>>> diff --git a/suricatta/server_hawkbit.h b/suricatta/server_hawkbit.h
>>> index aea4bb0..3b155f3 100644
>>> --- a/suricatta/server_hawkbit.h
>>> +++ b/suricatta/server_hawkbit.h
>>> @@ -44,6 +44,8 @@ typedef struct {
>>>        char *cached_file;
>>>        bool usetokentodwl;
>>>        unsigned int initial_report_resend_period;
>>> +    int server_status;
>>> +    time_t server_status_time;
>>>    } server_hawkbit_t;
>>>      extern server_hawkbit_t server_hawkbit;
>>>
>>
> 

Best regards,
Stefano Babic
diff mbox series

Patch

diff --git a/include/network_ipc.h b/include/network_ipc.h
index 1a3d957..84cd7b1 100644
--- a/include/network_ipc.h
+++ b/include/network_ipc.h
@@ -47,7 +47,8 @@  typedef enum {
 enum {
 	CMD_ACTIVATION,	/* this returns the answer if a SW can be activated */
 	CMD_CONFIG,
-	CMD_ENABLE	/* Enable or disable suricatta mode */
+	CMD_ENABLE,	/* Enable or disable suricatta mode */
+	CMD_GET_STATUS
 };
 
 enum run_type {
diff --git a/suricatta/server_hawkbit.c b/suricatta/server_hawkbit.c
index 805158d..2298119 100644
--- a/suricatta/server_hawkbit.c
+++ b/suricatta/server_hawkbit.c
@@ -543,7 +543,11 @@  static server_op_res_t server_get_device_info(channel_t *channel, channel_data_t
 		result = SERVER_EINIT;
 		goto cleanup;
 	}
-	if ((result = map_channel_retcode(channel->get(channel, (void *)channel_data))) !=
+
+	channel_op_res_t ch_response = channel->get(channel, (void *)channel_data);
+	server_hawkbit.server_status = ch_response;
+	server_hawkbit.server_status_time = time(NULL);
+	if ((result = map_channel_retcode(ch_response)) !=
 	    SERVER_OK) {
 		goto cleanup;
 	}
@@ -2026,6 +2030,19 @@  static server_op_res_t server_configuration_ipc(ipc_message *msg)
 	return SERVER_OK;
 }
 
+static server_op_res_t server_status_ipc(ipc_message *msg)
+{
+	TRACE("Received get status\n%s\n", msg->data.procmsg.buf);  // TODO clean-up
+
+	sprintf(msg->data.procmsg.buf,
+		"{\"server\":{\"status\":%d,\"time\":%ld}}",
+		server_hawkbit.server_status,
+		server_hawkbit.server_status_time);
+	msg->data.procmsg.len = strlen(msg->data.procmsg.buf);
+
+	return SERVER_OK;
+}
+
 server_op_res_t server_ipc(ipc_message *msg)
 {
 	server_op_res_t result = SERVER_OK;
@@ -2037,6 +2054,9 @@  server_op_res_t server_ipc(ipc_message *msg)
 	case CMD_CONFIG:
 		result = server_configuration_ipc(msg);
 		break;
+	case CMD_GET_STATUS:
+		result = server_status_ipc(msg);
+		break;
 	default:
 		result = SERVER_EERR;
 		break;
diff --git a/suricatta/server_hawkbit.h b/suricatta/server_hawkbit.h
index aea4bb0..3b155f3 100644
--- a/suricatta/server_hawkbit.h
+++ b/suricatta/server_hawkbit.h
@@ -44,6 +44,8 @@  typedef struct {
 	char *cached_file;
 	bool usetokentodwl;
 	unsigned int initial_report_resend_period;
+	int server_status;
+	time_t server_status_time;
 } server_hawkbit_t;
 
 extern server_hawkbit_t server_hawkbit;