freecwmp
[Top] [All Lists]

Re: patch for the freecwmp project

To: freecwmp@linux-mips.org
Subject: Re: patch for the freecwmp project
From: KALLEL Mohamed <mohamed.kallel@pivasoftware.com>
Date: Wed, 29 Aug 2012 08:51:05 +0100
In-reply-to: <20120828122841.GA8612@w500.iskon.local>
Original-recipient: rfc822;freecwmp@linux-mips.org
References: <50334294.9020508@pivasoftware.com> <20120828122841.GA8612@w500.iskon.local>
Sender: freecwmp-bounce@linux-mips.org
User-agent: Mozilla/5.0 (Windows NT 6.1; rv:14.0) Gecko/20120713 Thunderbird/14.0
Hi
It's pleasure to me contribute to your project and thank you for accepting the path
Please , see here after my response to your comments.


Le 28/08/2012 13:28, Luka Perkov a écrit :
Hi Mohamed,

On Tue, Aug 21, 2012 at 09:11:00AM +0100, KALLEL Mohamed wrote:
I'm Mohamed KALLEL and I'm interested in your freecwmp project.
Welcome aboard!

And I want to contribute in the development with the following patch
(patch attached with this email).
Thank you. I'll try to merge it this week, for now I'll give some quick
comments bellow.

The patch allow to support many events in the inform message. the
old behavior support only one event in the inform message
this patch allow to add the boot event (with the bootstarp event) in
the inform message.
the patch is based on the revision: *6d074ab*

Mohamed KALLEL


diff --git a/bin/Makefile.am b/bin/Makefile.am
index aa4db87..d7abfe2 100644
--- a/bin/Makefile.am
+++ b/bin/Makefile.am
@@ -7,6 +7,8 @@ freecwmpd_SOURCES =		\
 	../src/config.c		\
 	../src/cwmp/cwmp.h	\
 	../src/cwmp/cwmp.c	\
+	../src/cwmp/event.h	\
+	../src/cwmp/event.c	\
 	../src/cwmp/external.h	\
 	../src/cwmp/external.c	\
 	../src/http/b64.h	\
diff --git a/ext/openwrt/config/freecwmp b/ext/openwrt/config/freecwmp
index 05dd3be..e6f126d 100644
--- a/ext/openwrt/config/freecwmp
+++ b/ext/openwrt/config/freecwmp
@@ -2,7 +2,8 @@ config local
 	option interface eth0
 	option port 7547
 	option ubus_socket /var/run/ubus.sock
-	option event bootstrap
+	list event bootstrap
+	list event boot
I think we could leave this out and by default if there is no event list
we use this two options (bootstrap and boot).
we could add in the launcher script (freecwmp) a script which add the bootsrap event in case the acs address is changed and a boot event in case of the boot of the equipment. The both event could be added in the config (in the event list) before starting the freecwmp daemon. after the send of the event, the freecwmpd should remove the events from the config (c code) . this behaviour could be added in the future

 config acs
 	option scheme http
diff --git a/src/config.c b/src/config.c
index ad7a86e..abc621c 100644
--- a/src/config.c
+++ b/src/config.c
@@ -12,6 +12,7 @@
 
 #include "config.h"
 #include "cwmp/cwmp.h"
+#include "cwmp/event.h"
 
 static bool first_run = true;
 static struct uci_context *uci_ctx;
@@ -27,13 +28,14 @@ config_free_local(void) {
 	FREE(config->local->port);
 	FREE(config->local->ubus_socket);
 	FREE(config->local);
+	event_remove_all_events();
 }
 
 static int
 config_init_local(void)
 {
 	struct uci_section *s;
-	struct uci_element *e;
+	struct uci_element *e,*ie;
 
 	uci_foreach_element(&uci_freecwmp->sections, e) {
 		s = uci_to_section(e);
@@ -69,14 +71,20 @@ section_found:
 			goto next;
 		}
 
-		if (!strcmp((uci_to_option(e))->e.name, "event")) {
-			if (!strcasecmp("bootstrap", uci_to_option(e)->v.string))
-				config->local->event = BOOTSTRAP;
+		if (!strcmp((uci_to_option(e))->e.name, "event") && (uci_to_option(e))->type == UCI_TYPE_LIST) {
+			uci_foreach_element(&((uci_to_option(e))->v.list), ie)
+			{
+				if((ie != NULL)&&(ie->name!=NULL))
+				{
+					if (!strcasecmp("bootstrap", ie->name))
+						event_add_event(BOOTSTRAP,NULL);
 
-			if (!strcasecmp("boot", uci_to_option(e)->v.string))
-				config->local->event = BOOT;
+					if (!strcasecmp("boot", ie->name))
+						event_add_event(BOOT,NULL);
 
-			DD("freecwmp.@local[0].event=%s\n", uci_to_option(e)->v.string);
+					DD("freecwmp.@local[0].event=%s\n", ie->name);
I'm not sure if this output will be the same as the one from uci. I'll
double check.
I Know that the output is not the same as the one from uci. but this the quick solution I found for this kind of debug message. (It's just a debug message, so that why I did not look for a solution this)

+				}
+			}
 		}
 
 next:
diff --git a/src/config.h b/src/config.h
index 0bfc096..718d9bc 100644
--- a/src/config.h
+++ b/src/config.h
@@ -45,7 +45,6 @@ struct local {
 	char *interface;
 	char *port;
 	char *ubus_socket;
-	int event;
 };
 
 struct core_config {
diff --git a/src/cwmp/cwmp.c b/src/cwmp/cwmp.c
index 7ba851a..d6a0e49 100644
--- a/src/cwmp/cwmp.c
+++ b/src/cwmp/cwmp.c
@@ -12,6 +12,7 @@
 #include <libubox/uloop.h>
 
 #include "cwmp.h"
+#include "event.h"
 #include "external.h"
 #include "../freecwmp.h"
 #include "../config.h"
@@ -20,7 +21,7 @@
 
 static struct cwmp
 {
-	enum cwmp_event_code event_code;
+	struct list_head *event_list;
 	struct uloop_timeout connection_request_t;
 	struct uloop_timeout periodic_inform_t;
 	int8_t periodic_inform_enabled;
@@ -45,7 +46,7 @@ cwmp_init(void)
 	cwmp.acs_connection_required = 0;
 	cwmp.periodic_inform_enabled = 0;
 	cwmp.periodic_inform_interval = 0;
-	cwmp.event_code = config->local->event;
+	cwmp.event_list = &event_list;
 
 	status = cwmp_get_parameter_handler("InternetGatewayDevice.ManagementServer.PeriodicInformInterval", &c);
 	if (status == FC_SUCCESS && c) {
@@ -181,7 +182,7 @@ cwmp_periodic_inform(struct uloop_timeout *timeout)
 	if (cwmp.periodic_inform_enabled && cwmp.periodic_inform_interval) {
 		cwmp.periodic_inform_t.cb = cwmp_periodic_inform;
 		uloop_timeout_set(&cwmp.periodic_inform_t, cwmp.periodic_inform_interval * 1000);
-		cwmp.event_code = PERIODIC;
+		event_add_event(PERIODIC,NULL);
 	}
 
 	if (cwmp.periodic_inform_enabled)
@@ -405,7 +406,7 @@ cwmp_connection_request(void)
 
 	int8_t status;
 
-	cwmp.event_code = CONNECTION_REQUEST;
+	event_add_event(CONNECTION_REQUEST,NULL);
 	status = cwmp_inform();
 
 	FC_DEVEL_DEBUG("exit");
@@ -441,7 +442,7 @@ cwmp_add_notification(char *parameter, char *value)
 		n->value = strdup(value);
 	}
 
-	cwmp.event_code = VALUE_CHANGE;
+	event_add_event(VALUE_CHANGE,NULL);
 	if (!strncmp(c, "2", 1)) {
 		cwmp_inform();
 	}
@@ -488,7 +489,7 @@ cwmp_set_parameter_write_handler(char *name, char *value)
 	}
 
 	if((strcmp(name, "InternetGatewayDevice.ManagementServer.URL")) == 0) {
-		cwmp.event_code = VALUE_CHANGE;
+		event_add_event(VALUE_CHANGE,NULL);
 		cwmp.config_reload = true;
 		cwmp.acs_connection_required = 1; 
 	}
@@ -657,12 +658,23 @@ done:
 }
 
 char *
-cwmp_get_event_code(void)
+cwmp_get_event_code(enum cwmp_get_order order)
 {
 	FC_DEVEL_DEBUG("enter");
 
-	char *cwmp_inform_event_code;
-	switch (cwmp.event_code) {
+	static struct list_head *ilist = NULL;
+	struct event *event;
+	char *cwmp_inform_event_code = NULL;
+
+	if (order == FIRST || ilist == NULL)
+	{
Coding style tip:

	if (order == FIRST || ilist == NULL) {

That goes for all your if's and for's...

+		ilist = cwmp.event_list;
+	}
+	ilist = ilist->next;
+	if (ilist != cwmp.event_list)
+	{
+		event = list_entry (ilist,struct event,list);
+		switch (event->code) {
 		case BOOT:
 			cwmp_inform_event_code = "1 BOOT";
 			break;
@@ -682,6 +694,7 @@ cwmp_get_event_code(void)
 		default:
 			cwmp_inform_event_code = "0 BOOTSTRAP";
 			break;
+		}
 	}
 
 	FC_DEVEL_DEBUG("exit");
diff --git a/src/cwmp/cwmp.h b/src/cwmp/cwmp.h
index 947539c..a1b3d8b 100644
--- a/src/cwmp/cwmp.h
+++ b/src/cwmp/cwmp.h
@@ -27,6 +27,12 @@ enum cwmp_event_code
 	AUTONOMOUS_TRANSFER_COMPLETE 
 }; 
 
+enum cwmp_get_order
+{
+	FIRST = 0,
+	NEXT
+};
+
 struct notification {
 	struct list_head list;
 
@@ -55,7 +61,7 @@ int8_t cwmp_download_handler(char *url, char *size);
 int8_t cwmp_reboot_handler(void);
 int8_t cwmp_factory_reset_handler(void);
 int8_t cwmp_reload_changes(void);
-char * cwmp_get_event_code(void);
+char * cwmp_get_event_code(enum cwmp_get_order);
 int cwmp_get_retry_count(void);
 
 #endif
diff --git a/src/cwmp/event.c b/src/cwmp/event.c
index e69de29..561d413 100644
--- a/src/cwmp/event.c
+++ b/src/cwmp/event.c
GPLv2 license header is missing...

Also I'll see if this can go in some existing file.
I suggest to keep both file untill we finish the event behaviours . Because, as indicated in the standard, the management of the event is a bit complicated (Multiple event... , single event, more than 1 event in the inform message, pssibility to have 2 event list at the same time, a list which used in the current session with acs, and another list which used as queue of events, possibility to add mutex to manage events : session events and queue events...)

@@ -0,0 +1,33 @@
+#include "event.h"
+#include "cwmp.h"
+#include "../freecwmp.h"
+LIST_HEAD(event_list);
+
+void
+event_add_event(uint8_t code, char *commandKey)
Please name your variables like this command_key instead commandKey.

+{
+	struct event *event;
+
+	event = calloc(1,sizeof(struct event));
+	event->code = code;
+	event->commandKey = commandKey;
+	list_add (&(event->list), &(event_list));
+
+	FC_DEVEL_DEBUG("exit");
+}
+
+void
+event_remove_all_events()
+{
+	struct event *event;
+
+	FC_DEVEL_DEBUG("enter");
I'm doing some cleanup so I'll get rid of FC_DEVEL_DEBUG in the near
future. It's ok for now, I just wanted to give you heads up.

+
+	while (event_list.next != &event_list)
+	{
+		event = list_entry (event_list.next,struct event, list);
+		list_del(&event->list);
+		free(event);
+	}
+	FC_DEVEL_DEBUG("exit");
+}
diff --git a/src/cwmp/event.h b/src/cwmp/event.h
index e69de29..07afa8e 100644
--- a/src/cwmp/event.h
+++ b/src/cwmp/event.h
@@ -0,0 +1,18 @@
GPLv2 license header is missing...

+#ifndef _FREECWMP_EVENT_H__
+#define _FREECWMP_EVENT_H__
+
+#include <stdint.h>
+#include <libubox/uloop.h>
+
+struct event
+{
+	struct list_head list;
+	char *commandKey;
+	uint8_t code;
+};
+
+void event_add_event(uint8_t code, char *commandKey);
+void event_remove_all_events();
+
+extern struct list_head event_list;
+#endif
diff --git a/src/cwmp/messages.h b/src/cwmp/messages.h
index 046fc82..eea86d1 100644
--- a/src/cwmp/messages.h
+++ b/src/cwmp/messages.h
@@ -29,11 +29,7 @@
 			"<ProductClass/>"									\
 			"<SerialNumber/>"									\
 		"</DeviceId>"											\
-		"<Event soap_enc:arrayType=\"cwmp:EventStruct[1]\">"						\
-			"<EventStruct>"										\
-				"<EventCode/>"									\
-				"<CommandKey/>"									\
-			"</EventStruct>"									\
+		"<Event soap_enc:arrayType=\"cwmp:EventStruct[0]\">"						\
 		"</Event>"											\
 		"<MaxEnvelopes>1</MaxEnvelopes>"								\
 		"<CurrentTime/>"										\
@@ -88,6 +84,11 @@
 "</soap_env:Body>"												\
 "</soap_env:Envelope>"
 
+#define CWMP_INFORM_EVENT_STRUCT \
+"<EventStruct>"			\
+	"<EventCode/>"		\
+	"<CommandKey/>"		\
+"</EventStruct>"
 
 #define CWMP_RESPONSE_MESSAGE \
 "<?xml version=\"1.0\" encoding=\"UTF-8\" standalone=\"no\"?>"		\
diff --git a/src/xml/xml.c b/src/xml/xml.c
index 05424a5..c05cfc0 100644
--- a/src/xml/xml.c
+++ b/src/xml/xml.c
@@ -124,6 +124,40 @@ xml_exit(void)
 }
 
 int8_t
+xml_prepare_events_inform (mxml_node_t *tree)
+{
+	mxml_node_t *node, *event_node, *code_node;
+	char *c, buf[32];
+	uint8_t n = 0;
+
+
+	node = mxmlFindElement(tree, tree, "Event", NULL, NULL, MXML_DESCEND);
+
+	for (c = cwmp_get_event_code(FIRST); c!= NULL; c =  cwmp_get_event_code(NEXT))
+	{
+		event_node	= mxmlLoadString(NULL, CWMP_INFORM_EVENT_STRUCT, MXML_NO_CALLBACK);
+		if (!event_node)
+			return 0;
+		code_node	= mxmlFindElement(event_node, event_node, "EventCode", NULL, NULL, MXML_DESCEND);
+		if (!code_node)
+			return 0;
+		code_node	= mxmlNewText(code_node, 0, c);
+		if (!code_node)
+			return 0;
+		mxmlAdd (node,MXML_ADD_AFTER,MXML_ADD_TO_PARENT,event_node);
+		n++;
+	}
+
+	if (n)
+	{
+		sprintf (buf,"cwmp:EventStruct[%u]",n);
+		mxmlElementSetAttr(node, "soap_enc:arrayType", buf );	/* I - Attribute value */
+	}
+
+	return 1;
+}
+
+int8_t
 xml_prepare_inform_message(char **msg_out)
 {
 	FC_DEVEL_DEBUG("enter");
@@ -178,11 +212,7 @@ xml_prepare_inform_message(char **msg_out)
 	if (!busy_node)
 		goto error;
 
-	busy_node = mxmlFindElement(tree, tree, "EventCode", NULL, NULL, MXML_DESCEND);
-	if (!busy_node)
-		goto error;
-	busy_node = mxmlNewText(busy_node, 0, cwmp_get_event_code());
-	if (!busy_node)
+	if (!xml_prepare_events_inform (tree))
 		goto error;
 
 	busy_node = mxmlFindElement(tree, tree, "CurrentTime", NULL, NULL, MXML_DESCEND);
That would be it for now. I'll try and merge your patch this week.

I would also like to invite you to #freecwmp IRC channel on freenode.

Regards,
Luka

Regards
Mohamed

<Prev in Thread] Current Thread [Next in Thread>