So.. We'd have to make sure dynamically modifying the device description doesn't mess with the order (this is already done in RootDeviceFactory.set_description() and will be done in bug 703044). I guess it's enough to have a ordered list of known child names of device-node and check that in DescriptionFile.set_device_element(). Would also need to make RootDeviceFactory only modify the description via DescriptionFile.

I'll see what I can do.

Created attachment 247884 [details] [review]
DescriptionFile: create device elements if they do not exists

set_device_element() may be called with names of elements that do not
exists yet (non-required elements that are empty by default).

Create elements dynamically in set_device_element(). also, set dlna
namespace for X_DLNACAP and remove the X_DLNACAP element if the
contents are empty.

https://bugzilla.gnome.org/show_bug.cgi?id=703044

Created attachment 247885 [details] [review]
DescriptionFile: create device elements if they do not exists

set_device_element() may be called with names of elements that do not
exists yet (non-required elements that are empty by default).

Create elements dynamically in set_device_element(). also, set dlna
namespace for X_DLNACAP and remove the X_DLNACAP element if the
contents are empty.

https://bugzilla.gnome.org/show_bug.cgi?id=703044

Created attachment 247886 [details] [review]
xml templates: fix element order to match UDA 1.1

UPnP Device Architecture 1.1:
"The order of XML elements in device and service description documents
MUST adhere to the order as defined in the corresponding specification"

Note that this commit is not enough for compliance: when we dynamically
add optional elements, we must make sure they appear in the correct
place.

Created attachment 247887 [details] [review]
DescriptionFile: Preserve order when adding elements

UPnP Device Architecture 1.1 specifies the order of elements
in the device description. Preserve that order when dynamically
adding elements.

Created attachment 247888 [details] [review]
Only modify the description file from DescriptionFile

RootDeviceFactory used to modify the description xml directly. This is
not optimal as DescriptionFile now keeps the elements in the correct
order.

Move the icon element and service element creation code to
DescriptionFile, start using DescriptionFile public methods to modify
the description from RootDeviceFactory. Swap the order of ControlURL
and EventSubURL elements to match the specification.

This was a bit more involved than I thought... I can push a branch as well if you prefer that.


Also note that the first patch is really for bug 703044.

Review of attachment 247888 [details] [review]:

Thanks, I meant to clean this up but it wasn't necessary :)

::: src/librygel-core/rygel-root-device-factory.vala
@@ +107,3 @@
+        var udn = file.get_udn ();
+        if (udn == null || udn == "") {
+            file.set_udn (this.generate_random_udn ());

While you're at it, you can remove generate_random_udn and just use "uuid:" + UUID.get ()

Review of attachment 247887 [details] [review]:

Go ahead

Review of attachment 247886 [details] [review]:

Go ahead

Review of attachment 247885 [details] [review]:

Go ahead

Review of attachment 247888 [details] [review]:

Thanks, I meant to clean this up but it wasn't necessary :)

::: src/librygel-core/rygel-root-device-factory.vala
@@ +107,3 @@
+        var udn = file.get_udn ();
+        if (udn == null || udn == "") {
+            file.set_udn (this.generate_random_udn ());

While you're at it, you can remove generate_random_udn and just use "uuid:" + UUID.get ()

Created attachment 247896 [details] [review]
RootDeviceFactory: use UUID.get() from UUID.vapi

Separate patch to address comment 12.

Review of attachment 247888 [details] [review]:

Ok, go ahead

Review of attachment 247896 [details] [review]:

Go ahead

Attachment 247886 [details] pushed as 55b7d33 - xml templates: fix element order to match UDA 1.1
Attachment 247887 [details] pushed as c4362ed - DescriptionFile: Preserve order when adding elements
Attachment 247888 [details] pushed as c03cef8 - Only modify the description file from DescriptionFile
Attachment 247896 [details] pushed as c31909a - RootDeviceFactory: use UUID.get() from UUID.vapi

Review of attachment 247885 [details] [review]:

This one's committed as well, I just got confused after git-bz push failed to modify this: apparently I don't have the bugzilla permissions to change the status of the bug ??

probably.

woah, crap, I forgot to lecture you on commit messages :)