[WIB] Added methods to query for serialNumber and Camera parameters without starting device.

[larshg]

Without the need of "starting" the device.

To prepare for reducing the start() method to only do what it is supposed to do - start streaming?

[larshg]

Hmm - Just tested to see if I also got images, but I suddenly kept getting buffer overflow in the depth_packet_parser.

It seems that the magic1 marker in DepthSubPacketFooter is the last given command_seq_ number and hence, if you call one of either of the query methods, the magic1 marker would be 10 instead of the 9 at the current implementation...

[christiankerl]

can you verify that the magic1 changes to 11 or higher numbers, if you call the query methods multiple times?

[larshg]

Yes, I have tested with af for loop for 500 calls and it was increased equally.

[christiankerl]

ok, I guess we have to fix DepthPacketStreamParser :)

[christiankerl]

I'd move these methods to Freenect2DeviceImpl and change getIr/ColorCameraParams() to invoke them, if they don't have a valid value yet. the query serial method should be unnecessary as the serial number is already retrieved during device enumeration

[larshg]

@christiankerl aren't they already in Freenect2DeviceImpl? Should they not be virtual - Should I change the IR/ColorCameraParams to be pointers instead - to check if they have a valid value? It's not that easy to validate if the struct itself is valid?

Edit: Ahh - they should not be pure virtual in the Freenect2Device part?

[floe]

I just realized that this will break when merged on top of the updated ColorCameraParams structure - sorry. Could you please re-submit based on current master?

[floe]

I'm a bit confused: PRs #68 and #82 somehow seem to have mixed? I think this is because they share some commits. @larshg can you somehow separate those two again, or close #82 and re-submit (I think due to the registration stuff having been merged in the meantime, #82 needs to be edited anyway - sorry).

[floe]

I'm pretty certain we can now close this - although I'm not quite sure how, the functions implemented by this PR are already part of master:libfreenect2.cpp with slightly different names (getSerialNumber, getColor/IrCameraParams).

[larshg]

They functions you mention has been around for ages. What I was trying to implement was to query the parameters without having to start the device. Right now the values are all read out in the start method, which should only start the actual streaming? right now it does multiple things.

[floe]

Aaah, sorry. OK, now I get it. In this case, could you amend your PR so that your functions are used within Freenect2DeviceImpl::start? Otherwise, we'd have a pretty big chunk of duplicate code.

[larshg]

Yes ofc. But I was i doubt about how to proceed, as there is no reason to Query the device each time, but only the first time. So should I make the various structs as a pointer and make a check for null-pointer? or should we just query the device each time?

[floe]

I think we don't need to mess around with additional pointers; for the serial number, we could check if it's the empty string, for intrinsics, we could check if fx == 0.0.

So e.g. querySerialNumber doesn't return anything (void), but stores the result in serial_, and in getSerialNumber, add if (serial_.empty()) querySerialNumber(); before. Finally, in start(), we just call all three query functions so the fields are definitely populated as soon as the device is running.

Sounds reasonable?

[larshg]

Yes, also possible way to do it.

I just wanted to remove the Query/qet methods from the start method, so it in end only does one thing, ie start the streaming.

So when the registration is created, the getColorParams calls queryColor and afterwards you can start/stop, without doing any "new" readings of the params from the device.

ofc its minimal overhead in this and maybe its unreasonable to spent time on it?

[floe]

No, I think it is still reasonable to call them in start(), because you never know if some application might first start the device and then retrieve the serial number or intrinsics later, possibly even from another thread. And if you submit the USB query commands right in the middle of the regular data stream, who knows what other ugly USB3 issues might suddenly happen...

[larshg]

If we should still call them in start I don't see any reason to move it out from the start. So I'll close this one.