uORB::Publication: automatically obtain ORB_QUEUE_LENGTH from the structure

[shawnfeng0]

Use the C++ template technique to get the ORB_QUEUE_LENGTH in the structure, and return the default value if it does not contain ORB_QUEUE_LENGTH.

In this way, there is no need to remember whether a topic has a queue, uORB::Publication will automatically configure the queue size according to the topic in the *.msg file.

[dagar]

There's a bit of a hack on destruction to be aware of.
https://github.com/PX4/Firmware/blob/b14d0e43265b25a49905c9d543756ed442cbbb2c/src/modules/uORB/Publication.hpp#L67-L71

This is because of standalone uses publishing a vehicle_command by stack allocating uORB::PublicationQueued and we need the vehicle_command to still exist. Maybe we need a slightly different approach for this use case?

[shawnfeng0]

It may be feasible to put this destructor in Publication and PublicationMulti processing, how about it?
PublicationBase does not perform any functions, but only provides public interfaces and data.

But I think this use case affects the consistency of architecture performance, maybe we should delete this use case later.

I think no matter whether the broadcaster exists or not, the subscribers should be able to continue to get the unobtained data and make the system behave consistently.

[dagar]

I think no matter whether the broadcaster exists or not, the subscribers should be able to continue to get the unobtained data and make the system behave consistently.

I think what could work is leaving it be (skipping unadvertise) if any data was actually published. If the uORB::DeviceNode never published anything and doesn't have any subscribers then it should be deleted on unadvertise.

Later the entire uORB shutdown/cleanup story needs to be reviewed as it's largely been ignored due to most people turning off their drones by pulling power abruptly.

[shawnfeng0]

Of course, the data should be kept in DeviceNode.

My thoughts are a little different:

Current design:
If the publisher executes unadvertise(), the subscriber will not get the data.
Publishers with queues are not allowed to unadvertise() because the data in the queue is still useful.

Can be modified to:
The publisher executes unadvertise(), which does not affect the subscribers who have subscribed to continue to obtain unobtained data, whether there is only one or there is a queue of data. (Consistent behavior)

This will be another pull request.

[shawnfeng0]

By the way, in the current design, if a DeviceNode is published by multiple publishers, as long as one of the publishers is destroyed, the DeviceNode will be marked as unadvertised. (Need to record the number of publishers.)

[shawnfeng0]

Can this modification be merged? The unadvertise related issues mentioned above will be a new pull request.

[bkueng]

Thanks

[lukegluke]

This commits leads to specific factor.

[I'm working with custom board and project based on PX4]
I had LedsSimple driver that works in the same way as rgbled and calls _led_controller.update in Run().

in rcS right after this drivers starts I call led_control cmd three times in a row:

led_control off -p 0                 #turn off all leds
led_control on -l 0 -p 0             #turn on one led with 0 priority
led_control blink -n 3 -s fast -p 2  #blink all leds three times

before (when PublicationQueued was used) after blinking there is on led stay lighting as expected, now it doesn't.
Debugging showed that led_control cmd has time to publish all three orb messages in queue, before driver reaches _led_controller.update call and led controller subscribes to topic - so generation is already 3 for this moment and it reads only last message - blinking.

The workaround that works in my case: call first led_control cmd before starting LedsSimple driver. So than when LedController is created _led_control_sub will be initially subscribed (in its constructor) to already advertised topic.

But as I see the PX4 boards probably don't have such code that would call led_control/tune_control or other related commands in row on boot, right? Anyway just a warning.

[shawnfeng0]

@lukegluke
That is due to this #15709
After subscribing, current subscribers can only get one old data of before subscription.

PX4-Autopilot/src/modules/uORB/uORBDeviceNode.cpp

Lines 547 to 557 in 66edc82

	unsigned uORB::DeviceNode::get_initial_generation()
	{
	ATOMIC_ENTER;
	// If there any previous publications allow the subscriber to read them
	unsigned generation = _generation.load() - (_data_valid ? 1 : 0);
	ATOMIC_LEAVE;
	return generation;
	}

I think the new data should be used when the system starts running, and the queue buffer can be used later.

[lukegluke]

Yes, I saw this. And it is also due to Subscription will not subscribe until topic will be advertise:

PX4-Autopilot/src/modules/uORB/Subscription.cpp

Line 58 in 66edc82

if (!device_master->deviceNodeExists(_orb_id, _instance)) {