Re: [PATCH_v3 01/04] android/hid: Implement hid get protocol in daemon

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Johan,

On 11/05/2013 03:10 PM, Johan Hedberg wrote:
Hi Ravi,

On Tue, Nov 05, 2013, Ravi kumar Veeramally wrote:
+static gboolean ctrl_io_watch_cb(GIOChannel *chan, gpointer data)
+{
+	struct hid_device *dev = data;
+	int fd, bread;
+	uint8_t buf[UHID_DATA_MAX];
+
+	DBG("");
+
+	fd = g_io_channel_unix_get_fd(chan);
+	bread = read(fd, buf, sizeof(buf));
+	if (bread < 0) {
+		error("read: %s(%d)", strerror(errno), -errno);
+		return TRUE;
+	}
+
+	switch (dev->last_hid_msg) {
+	case HID_MSG_GET_PROTOCOL:
+		bt_hid_notify_proto_mode(dev, buf, bread);
+		break;
+	default:
+		DBG("unhandled hid msg type 0x%02x", dev->last_hid_msg);
+	}
This doesn't really make sense to me. If you only set last_hid_msg when
you send a code that you do support, then why would the value of
last_hid_msg ever contain a type that you do not support? (assuming you
always add an entry to this switch statement in the same patch that you
add a corresponding write for the type).

Also, since you don't seem to be using last_hid_msg for anything else
than printing this debug message, I'm wondering is there really any
value for it? Previously (based on our IRC) discussion I understood that
it had some actual functional value that helped determine what to send
to the HAL, but now I'm not seeing it anywhere in the patch. I might
have missed it though (in which case please enlighten me :)
 I don't know if I understand you correctly, but at the end of all patches
 it looks like this.
         switch (dev->last_hid_msg) {
        case HID_MSG_GET_PROTOCOL:
        case HID_MSG_SET_PROTOCOL:
                bt_hid_notify_proto_mode(dev, buf, bread);
                break;
        case HID_MSG_GET_REPORT:
                bt_hid_notify_get_report(dev, buf, bread);
                break;
        default:
                DBG("unhandled hid msg type 0x%02x", dev->last_hid_msg);
        }

based on last_hid_msg switch case, it will call respective function,
default statement is for debug purpose if we miss something from hid device.

Thanks,
Ravi.
--
To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Bluez Devel]     [Linux Wireless Networking]     [Linux Wireless Personal Area Networking]     [Linux ATH6KL]     [Linux USB Devel]     [Linux Media Drivers]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Big List of Linux Books]

  Powered by Linux