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

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

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

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.

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

807 + int get_fd() const;

I think we'd generally expect this to be called just ‘fd()’?

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

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.

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

739 + auto status = ::poll(&pfd, 1, timeout.count());
740 +
741 + if (status > 0)
742 + return true;
743 + if (status == 0)
744 + return false;
745 +
746 + // TODO: What to do in case of error? ~racarr
747 + return false;
748 +}

This should probably try polling(!) again while status == EINTR.

Other errors should probably be runtime exceptions; none of the other errors poll can return (EFAULT, EINVAL, ENOMEM) are going to go away without explicit action.

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