Mir

Code review comment for lp:~robertcarr/mir/receive-input-in-client

Revision history for this message
Robert Carr (robertcarr) wrote :

>>> In general: at least a little bit of Doxygen comments for the classes you add, please! ☺

I tried to add some helpful comments!

>> 716 + droidinput::status_t status;
>> 717 + if((status = input_consumer->consume(&event_factory, consume_batches,
>> 718 + default_frame_time, &event_sequence_id, &android_event)) != droidinput::WOULD_BLOCK)

>> Status is set but never used.

Fixed

>> 807 + int get_fd() const;

Fixed

------------------------

>> 822 + static const bool consume_batches = true;
>> 823 + static const bool do_not_consume_batches = false;
...
>> 825 + static const bool handle_event = true;
>> 826 + static const bool do_not_handle_event = false;

>> As far as I'm aware we've not been using this pattern elsewhere in the code? I'm not particularly against >> it, though.

These come from a long series of reviews on an original version of AndroidInputReceiver back in november...I do not have strong opinions though I do think they make the implementation read in a rather literate style.

-------------------------

>> I'm not comfortable letting this in while we're polling every 10msec for input.

The custom poll implementation is replaced with a droidinput::Looper, this way the poll is indefinite, but may still be woken by a wake pipe from another thread.

« Back to merge proposal