Merge lp://staging/~chasedouglas/frame/v2 into lp://staging/frame

Every single comment I have made about accessor functions vs generic property getter/setter applies here too. In fact it applies even stronger. As an example, the Axis class has no use for a generic setter. None. Zilch. Zero. There are no new properties that can magically appear at runtime. They are all specified by the kernel interface, the type enum and they are all required arguments of the constructor. All it does is complexify the code and circumvent the type system making the entire thing more fragile.

And if opaque getters and setters are to be used, they better be used everywhere and always. For example, UFTouchClass has getters and setters for id and state but not others. Which is it gonna be?

UFHandleClass requires an Initialize call before it is usable. This is wrong. Either this needs to be done in the constructor or a static factory. It might be sensible not to use exceptions in this case since Frame is being used as a C library. I'm not quite sure what happens if an exception escapes the boundary.

There are multiple duplicate declarations of SharedUFDeviceClass, SharedUFDeviceClass etc. There can be only one.

The PutEvent/GetEvent interface is a bit cryptic. Why is it called PutEvent if it just deletes the event. Should it not be ReleaseEvent? "Put" in a container object implies adding new stuff for further use. Actually, the entire interface seems a bit fishy to me. Could you give an example on how you envision it will be used and how it differs from the current API.

If this is going to be C++0x, then these sorts of structures:

  for (unsigned int i = 0; i < prev_->touches_array_.size(); ++i) {
    const SharedUFTouchClass& prev_touch = prev_->touches_array_[i];

should be replaced with auto iterators.

And so on and so on. I'm going to mark this as rejected simply to signal that there is a lot of work to be done.

> Every single comment I have made about accessor functions vs generic property
> getter/setter applies here too. In fact it applies even stronger. As an
> example, the Axis class has no use for a generic setter. None. Zilch. Zero.
> There are no new properties that can magically appear at runtime. They are all
> specified by the kernel interface, the type enum and they are all required
> arguments of the constructor. All it does is complexify the code and
> circumvent the type system making the entire thing more fragile.

Note that there are no setters in the api.

One can reason that the axis object is not going to be extended, so I have converted frame_axis_get_property() into four separate static accessor function.

The rest of the objects may be extended in the future, and they could be extended at runtime. Leaving them as generic opaque getters allows for better forward compatibility.

> And if opaque getters and setters are to be used, they better be used
> everywhere and always. For example, UFTouchClass has getters and setters for
> id and state but not others. Which is it gonna be?

Touches are retrieved through an indexed lookup because there's no other way to retrieve them from the frame object.

Axes can be retrieved through an indexed lookup for the same reason until you know what axis types are available for the device. Once you have looped through the available axes you can get an axis directly by its type.

These are special cases because the objects are part of arrays.

> UFHandleClass requires an Initialize call before it is usable. This is wrong.
> Either this needs to be done in the constructor or a static factory. It might
> be sensible not to use exceptions in this case since Frame is being used as a
> C library. I'm not quite sure what happens if an exception escapes the
> boundary.

It has been converted to a factor function. Additionally, frame_x11_new() now catches any exceptions and returns UFStatusErrorResources.

> There are multiple duplicate declarations of SharedUFDeviceClass,
> SharedUFDeviceClass etc. There can be only one.

These are essentially forward declarations of the typedefs. If you don't forward declare them, you will need to include the appropriate header files. If you attempt to do that (which I tried to do), you get into a header recursion and end up with undefined type errors.

This leaves us with two possibilities: forgo the typedefs and use the exact definitions, or leave the forward declarations of typedefs. Note that the typedefs cannot get out of sync or else you will get compiler errors stating there is a conflicting declaration. I have decided to leave them as it makes the code easier to read.

> The PutEvent/GetEvent interface is a bit cryptic. Why is it called PutEvent if
> it just deletes the event. Should it not be ReleaseEvent? "Put" in a container
> object implies adding new stuff for further use. Actually, the entire
> interface seems a bit fishy to me. Could you give an example on how you
> envision it will be used and how it differs from the current API.

In linux, and I believe elsewhere too, some reference counting APIs use "get" and "put" to increment and decrement the referen...

I want to note that this api is nowhere near final. What I propose is instead of merging this into lp:utouch-frame, we merge it into lp:utouch-frame/v2. We continually merge any changes from trunk (which should be few if any), and we switch the daily ppa over to build from lp:utouch-frame/v2. We can then have a final merge review before merging lp:utouch-frame/v2 into lp:utouch-frame.

During this time, we'll be able to kick the tires on the api. If we find certain things to be cumbersome, we can change them. From this point of view, this merge proposal should be about finding and fixing implementation issues and the direction of the api. For example, if the api structure is reasonable, but there's a question as to what should have static accessors and what should have generic property getters, we can address those later on before merging back to trunk.

Forward declarations have one of the following two forms:

class Foo;
struct Foo;

What we have in the code is a type declaration. They must _not_ be duplicated. Ever. In this particular case an easy fix is to have a header such as basic_definitions.h which does not include anything and has the following contents:

class UFTouch;
// other forward declared classes here

typedef std::shared_ptr<const UFTouch_> SharedUFTouch_;
// other type declaration thingies here.

It can be included anywhere, is short, conflict-free and does not cause compilation slowdown.

The X event object is created in utouch-frame-test-x11 but it is released in UFHandleX11_::InjectEvent. This is an ownership error. The one who created the event should have the responsibility of destroying it as well.

The main function in x11 test app is too long. It should be split into smaller, logical groups (setup, processing, teardown or something similar).

PRINT_PROPERTY is quite long, it should really be a function.

The header file of x11-test lists Henrik as the author.