On 04/10/2014 11:03 AM, Jakub Hrozek wrote:
The attached patch implements introspecting the sbus interfaces as
tracked by #2234.
There is one part of the patch I dislike, but I wanted to get other
opinions, too -- the discard_const in sbus_message_handler(). I was
going back and forth on whether sbus_introspect() should be an sbus
handler like the other handlers or whether I should special case it (and
then pass the interface directly and not through a void pointer).
Special casing the introspection might be a bit cleaner, but would cause
some duplication in sbus_message_handler().
In my opinion it is correct to make it a standard sbus handler. I think
that it is ok to use discard_const() in this situation, however it can
be avoided. Cleaner solution would be to create a new context for
private data, e.g.:
struct sbus_introspect_ctx {
const struct sbus_interface_meta *iface;
};
The other question I have is whether there should be a way to mark
an
interface as not introspectable? I can't think of a reason, so by
default all interfaces can be introspected.
We should deal with this question when we actually hit an interface that
must not be introspectable for some reason.
The introspection also doesn't generate DocString annotations
yet, I
will send a patch for that separately.
The code looks good and it works splendidly! Good job!
I'm leaving the discard_const issue on your decision, I don't think it
is really necessary to avoid it in this case.
It's an ack from me.