* [PATCH BlueZ 1/2] Revert "input/hog: Remove HID device after HoG device disconnects"
@ 2020-12-09 1:00 Sonny Sasaka
2020-12-09 1:00 ` [PATCH BlueZ 2/2] input/hog: Cache the HID report map Sonny Sasaka
2020-12-09 1:56 ` [BlueZ,1/2] Revert "input/hog: Remove HID device after HoG device disconnects" bluez.test.bot
0 siblings, 2 replies; 7+ messages in thread
From: Sonny Sasaka @ 2020-12-09 1:00 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Sonny Sasaka, Alain Michaud
This reverts commit d6cafa1f0c3ea1989f8a580e52f418b0998a3552.
In commit d6cafa1f0c3e ("input/hog: Remove HID device after HoG device
disconnects"), the bt_hog structure is destroyed in order to fix a bug
where the UHID connection is not destroyed. This fix has the cost of
increasing reconnection time because every reconnection would need to
re-read the report map again. An improvement to this fix is, instead of
removing the bt_hog structure, we can just destroy the UHID with
UHID_DESTROY event and use the existing bt_hog structure to keep the
cache of the report map to avoid re-reading the report map at
reconnection.
Reviewed-by: Alain Michaud <alainm@chromium.org>
---
profiles/input/hog.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/profiles/input/hog.c b/profiles/input/hog.c
index 91de4c70f..d50b82321 100644
--- a/profiles/input/hog.c
+++ b/profiles/input/hog.c
@@ -207,8 +207,6 @@ static int hog_disconnect(struct btd_service *service)
struct hog_device *dev = btd_service_get_user_data(service);
bt_hog_detach(dev->hog);
- bt_hog_unref(dev->hog);
- dev->hog = NULL;
btd_service_disconnecting_complete(service, 0);
--
2.26.2
^ permalink raw reply related [flat|nested] 7+ messages in thread* [PATCH BlueZ 2/2] input/hog: Cache the HID report map 2020-12-09 1:00 [PATCH BlueZ 1/2] Revert "input/hog: Remove HID device after HoG device disconnects" Sonny Sasaka @ 2020-12-09 1:00 ` Sonny Sasaka 2020-12-09 1:57 ` Luiz Augusto von Dentz 2020-12-09 1:56 ` [BlueZ,1/2] Revert "input/hog: Remove HID device after HoG device disconnects" bluez.test.bot 1 sibling, 1 reply; 7+ messages in thread From: Sonny Sasaka @ 2020-12-09 1:00 UTC (permalink / raw) To: linux-bluetooth; +Cc: Sonny Sasaka, Alain Michaud To optimize BLE HID devices reconnection response, we can cache the report map so that the subsequent reconnections do not need round trip time to read the report map. Reviewed-by: Alain Michaud <alainm@chromium.org> --- profiles/input/hog-lib.c | 142 ++++++++++++++++++++++++++------------- 1 file changed, 96 insertions(+), 46 deletions(-) diff --git a/profiles/input/hog-lib.c b/profiles/input/hog-lib.c index ee811d301..1e198ea64 100644 --- a/profiles/input/hog-lib.c +++ b/profiles/input/hog-lib.c @@ -95,6 +95,8 @@ struct bt_hog { struct queue *bas; GSList *instances; struct queue *gatt_op; + uint8_t report_map[HOG_REPORT_MAP_MAX_SIZE]; + ssize_t report_map_len; }; struct report { @@ -276,6 +278,8 @@ static void find_included(struct bt_hog *hog, GAttrib *attrib, free(req); } +static void uhid_create(struct bt_hog *hog); + static void report_value_cb(const guint8 *pdu, guint16 len, gpointer user_data) { struct report *report = user_data; @@ -924,57 +928,17 @@ static char *item2string(char *str, uint8_t *buf, uint8_t len) return str; } -static void report_map_read_cb(guint8 status, const guint8 *pdu, guint16 plen, - gpointer user_data) +static void uhid_create(struct bt_hog *hog) { - struct gatt_request *req = user_data; - struct bt_hog *hog = req->user_data; - uint8_t value[HOG_REPORT_MAP_MAX_SIZE]; struct uhid_event ev; - ssize_t vlen; - char itemstr[20]; /* 5x3 (data) + 4 (continuation) + 1 (null) */ - int i, err; GError *gerr = NULL; + int i, err; - destroy_gatt_req(req); - - DBG("HoG inspecting report map"); - - if (status != 0) { - error("Report Map read failed: %s", att_ecode2str(status)); - return; - } - - vlen = dec_read_resp(pdu, plen, value, sizeof(value)); - if (vlen < 0) { - error("ATT protocol error"); + if (!hog->report_map_len) { + warn("Failed to initiate UHID_CREATE without report map"); return; } - DBG("Report MAP:"); - for (i = 0; i < vlen;) { - ssize_t ilen = 0; - bool long_item = false; - - if (get_descriptor_item_info(&value[i], vlen - i, &ilen, - &long_item)) { - /* Report ID is short item with prefix 100001xx */ - if (!long_item && (value[i] & 0xfc) == 0x84) - hog->has_report_id = TRUE; - - DBG("\t%s", item2string(itemstr, &value[i], ilen)); - - i += ilen; - } else { - error("Report Map parsing failed at %d", i); - - /* Just print remaining items at once and break */ - DBG("\t%s", item2string(itemstr, &value[i], vlen - i)); - break; - } - } - - /* create uHID device */ memset(&ev, 0, sizeof(ev)); ev.type = UHID_CREATE; @@ -1004,8 +968,8 @@ static void report_map_read_cb(guint8 status, const guint8 *pdu, guint16 plen, ev.u.create.version = hog->version; ev.u.create.country = hog->bcountrycode; ev.u.create.bus = BUS_BLUETOOTH; - ev.u.create.rd_data = value; - ev.u.create.rd_size = vlen; + ev.u.create.rd_data = hog->report_map; + ev.u.create.rd_size = hog->report_map_len; err = bt_uhid_send(hog->uhid, &ev); if (err < 0) { @@ -1018,6 +982,62 @@ static void report_map_read_cb(guint8 status, const guint8 *pdu, guint16 plen, bt_uhid_register(hog->uhid, UHID_SET_REPORT, set_report, hog); hog->uhid_created = true; +} + +static void report_map_read_cb(guint8 status, const guint8 *pdu, guint16 plen, + gpointer user_data) +{ + struct gatt_request *req = user_data; + struct bt_hog *hog = req->user_data; + ssize_t vlen; + char itemstr[20]; /* 5x3 (data) + 4 (continuation) + 1 (null) */ + int i; + + destroy_gatt_req(req); + + DBG("HoG inspecting report map"); + + if (status != 0) { + error("Report Map read failed: %s", att_ecode2str(status)); + return; + } + + vlen = dec_read_resp(pdu, plen, hog->report_map, + sizeof(hog->report_map)); + if (vlen < 0) { + error("ATT protocol error"); + return; + } + + hog->report_map_len = vlen; + + DBG("Report MAP:"); + for (i = 0; i < vlen;) { + ssize_t ilen = 0; + bool long_item = false; + + if (get_descriptor_item_info(&hog->report_map[i], vlen - i, + &ilen, &long_item)) { + /* Report ID is short item with prefix 100001xx */ + if (!long_item && (hog->report_map[i] & 0xfc) == 0x84) + hog->has_report_id = TRUE; + + DBG("\t%s", item2string(itemstr, &hog->report_map[i], + ilen)); + + i += ilen; + } else { + error("Report Map parsing failed at %d", i); + + /* Just print remaining items at once and break */ + DBG("\t%s", item2string(itemstr, &hog->report_map[i], + vlen - i)); + break; + } + } + + /* create uHID device */ + uhid_create(hog); DBG("HoG created uHID device"); } @@ -1602,6 +1622,12 @@ bool bt_hog_attach(struct bt_hog *hog, void *gatt) bt_hog_attach(instance, gatt); } + /* Try to initiate UHID_CREATE if we already have the report map to + * avoid re-reading the report map from the peer device. + */ + if (hog->report_map_len > 0) + uhid_create(hog); + if (!hog->uhid_created) { DBG("HoG discovering characteristics"); if (hog->attr) @@ -1627,6 +1653,29 @@ bool bt_hog_attach(struct bt_hog *hog, void *gatt) return true; } +static void uhid_destroy(struct bt_hog *hog) +{ + int err; + struct uhid_event ev; + + if (!hog->uhid_created) + return; + + bt_uhid_unregister_all(hog->uhid); + + memset(&ev, 0, sizeof(ev)); + ev.type = UHID_DESTROY; + + err = bt_uhid_send(hog->uhid, &ev); + + if (err < 0) { + error("bt_uhid_send: %s", strerror(-err)); + return; + } + + hog->uhid_created = false; +} + void bt_hog_detach(struct bt_hog *hog) { GSList *l; @@ -1660,6 +1709,7 @@ void bt_hog_detach(struct bt_hog *hog) queue_foreach(hog->gatt_op, (void *) cancel_gatt_req, NULL); g_attrib_unref(hog->attrib); hog->attrib = NULL; + uhid_destroy(hog); } int bt_hog_set_control_point(struct bt_hog *hog, bool suspend) -- 2.26.2 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH BlueZ 2/2] input/hog: Cache the HID report map 2020-12-09 1:00 ` [PATCH BlueZ 2/2] input/hog: Cache the HID report map Sonny Sasaka @ 2020-12-09 1:57 ` Luiz Augusto von Dentz 2020-12-11 23:31 ` Sonny Sasaka 0 siblings, 1 reply; 7+ messages in thread From: Luiz Augusto von Dentz @ 2020-12-09 1:57 UTC (permalink / raw) To: Sonny Sasaka; +Cc: linux-bluetooth@vger.kernel.org, Alain Michaud Hi Sonny, On Tue, Dec 8, 2020 at 5:16 PM Sonny Sasaka <sonnysasaka@chromium.org> wrote: > > To optimize BLE HID devices reconnection response, we can cache the > report map so that the subsequent reconnections do not need round trip > time to read the report map. While the idea is pretty good we need to find a way to persist it across reboots/restarts. In theory we could save the value on the gatt_db instance and then make changes to device.c:store_desc to attempt to read if there is a value stored in the db save it on file as well so the next time the cache is loaded it should restore the report map, or any other descriptor that have its value saved in the database. That said we may need to introduce some sort of property for values stored in the db since normally the values store in the database shall not be persisted on file, in fact it would cause a lot of fime access updating the cache if we were to trigger updates everytime the db is updated with a value. > Reviewed-by: Alain Michaud <alainm@chromium.org> > > --- > profiles/input/hog-lib.c | 142 ++++++++++++++++++++++++++------------- > 1 file changed, 96 insertions(+), 46 deletions(-) > > diff --git a/profiles/input/hog-lib.c b/profiles/input/hog-lib.c > index ee811d301..1e198ea64 100644 > --- a/profiles/input/hog-lib.c > +++ b/profiles/input/hog-lib.c > @@ -95,6 +95,8 @@ struct bt_hog { > struct queue *bas; > GSList *instances; > struct queue *gatt_op; > + uint8_t report_map[HOG_REPORT_MAP_MAX_SIZE]; > + ssize_t report_map_len; > }; > > struct report { > @@ -276,6 +278,8 @@ static void find_included(struct bt_hog *hog, GAttrib *attrib, > free(req); > } > > +static void uhid_create(struct bt_hog *hog); > + > static void report_value_cb(const guint8 *pdu, guint16 len, gpointer user_data) > { > struct report *report = user_data; > @@ -924,57 +928,17 @@ static char *item2string(char *str, uint8_t *buf, uint8_t len) > return str; > } > > -static void report_map_read_cb(guint8 status, const guint8 *pdu, guint16 plen, > - gpointer user_data) > +static void uhid_create(struct bt_hog *hog) > { > - struct gatt_request *req = user_data; > - struct bt_hog *hog = req->user_data; > - uint8_t value[HOG_REPORT_MAP_MAX_SIZE]; > struct uhid_event ev; > - ssize_t vlen; > - char itemstr[20]; /* 5x3 (data) + 4 (continuation) + 1 (null) */ > - int i, err; > GError *gerr = NULL; > + int i, err; > > - destroy_gatt_req(req); > - > - DBG("HoG inspecting report map"); > - > - if (status != 0) { > - error("Report Map read failed: %s", att_ecode2str(status)); > - return; > - } > - > - vlen = dec_read_resp(pdu, plen, value, sizeof(value)); > - if (vlen < 0) { > - error("ATT protocol error"); > + if (!hog->report_map_len) { > + warn("Failed to initiate UHID_CREATE without report map"); > return; > } > > - DBG("Report MAP:"); > - for (i = 0; i < vlen;) { > - ssize_t ilen = 0; > - bool long_item = false; > - > - if (get_descriptor_item_info(&value[i], vlen - i, &ilen, > - &long_item)) { > - /* Report ID is short item with prefix 100001xx */ > - if (!long_item && (value[i] & 0xfc) == 0x84) > - hog->has_report_id = TRUE; > - > - DBG("\t%s", item2string(itemstr, &value[i], ilen)); > - > - i += ilen; > - } else { > - error("Report Map parsing failed at %d", i); > - > - /* Just print remaining items at once and break */ > - DBG("\t%s", item2string(itemstr, &value[i], vlen - i)); > - break; > - } > - } > - > - /* create uHID device */ > memset(&ev, 0, sizeof(ev)); > ev.type = UHID_CREATE; > > @@ -1004,8 +968,8 @@ static void report_map_read_cb(guint8 status, const guint8 *pdu, guint16 plen, > ev.u.create.version = hog->version; > ev.u.create.country = hog->bcountrycode; > ev.u.create.bus = BUS_BLUETOOTH; > - ev.u.create.rd_data = value; > - ev.u.create.rd_size = vlen; > + ev.u.create.rd_data = hog->report_map; > + ev.u.create.rd_size = hog->report_map_len; > > err = bt_uhid_send(hog->uhid, &ev); > if (err < 0) { > @@ -1018,6 +982,62 @@ static void report_map_read_cb(guint8 status, const guint8 *pdu, guint16 plen, > bt_uhid_register(hog->uhid, UHID_SET_REPORT, set_report, hog); > > hog->uhid_created = true; > +} > + > +static void report_map_read_cb(guint8 status, const guint8 *pdu, guint16 plen, > + gpointer user_data) > +{ > + struct gatt_request *req = user_data; > + struct bt_hog *hog = req->user_data; > + ssize_t vlen; > + char itemstr[20]; /* 5x3 (data) + 4 (continuation) + 1 (null) */ > + int i; > + > + destroy_gatt_req(req); > + > + DBG("HoG inspecting report map"); > + > + if (status != 0) { > + error("Report Map read failed: %s", att_ecode2str(status)); > + return; > + } > + > + vlen = dec_read_resp(pdu, plen, hog->report_map, > + sizeof(hog->report_map)); > + if (vlen < 0) { > + error("ATT protocol error"); > + return; > + } > + > + hog->report_map_len = vlen; > + > + DBG("Report MAP:"); > + for (i = 0; i < vlen;) { > + ssize_t ilen = 0; > + bool long_item = false; > + > + if (get_descriptor_item_info(&hog->report_map[i], vlen - i, > + &ilen, &long_item)) { > + /* Report ID is short item with prefix 100001xx */ > + if (!long_item && (hog->report_map[i] & 0xfc) == 0x84) > + hog->has_report_id = TRUE; > + > + DBG("\t%s", item2string(itemstr, &hog->report_map[i], > + ilen)); > + > + i += ilen; > + } else { > + error("Report Map parsing failed at %d", i); > + > + /* Just print remaining items at once and break */ > + DBG("\t%s", item2string(itemstr, &hog->report_map[i], > + vlen - i)); > + break; > + } > + } > + > + /* create uHID device */ > + uhid_create(hog); > > DBG("HoG created uHID device"); > } > @@ -1602,6 +1622,12 @@ bool bt_hog_attach(struct bt_hog *hog, void *gatt) > bt_hog_attach(instance, gatt); > } > > + /* Try to initiate UHID_CREATE if we already have the report map to > + * avoid re-reading the report map from the peer device. > + */ > + if (hog->report_map_len > 0) > + uhid_create(hog); > + > if (!hog->uhid_created) { > DBG("HoG discovering characteristics"); > if (hog->attr) > @@ -1627,6 +1653,29 @@ bool bt_hog_attach(struct bt_hog *hog, void *gatt) > return true; > } > > +static void uhid_destroy(struct bt_hog *hog) > +{ > + int err; > + struct uhid_event ev; > + > + if (!hog->uhid_created) > + return; > + > + bt_uhid_unregister_all(hog->uhid); > + > + memset(&ev, 0, sizeof(ev)); > + ev.type = UHID_DESTROY; > + > + err = bt_uhid_send(hog->uhid, &ev); > + > + if (err < 0) { > + error("bt_uhid_send: %s", strerror(-err)); > + return; > + } > + > + hog->uhid_created = false; > +} > + > void bt_hog_detach(struct bt_hog *hog) > { > GSList *l; > @@ -1660,6 +1709,7 @@ void bt_hog_detach(struct bt_hog *hog) > queue_foreach(hog->gatt_op, (void *) cancel_gatt_req, NULL); > g_attrib_unref(hog->attrib); > hog->attrib = NULL; > + uhid_destroy(hog); > } > > int bt_hog_set_control_point(struct bt_hog *hog, bool suspend) > -- > 2.26.2 > -- Luiz Augusto von Dentz ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH BlueZ 2/2] input/hog: Cache the HID report map 2020-12-09 1:57 ` Luiz Augusto von Dentz @ 2020-12-11 23:31 ` Sonny Sasaka 2020-12-12 1:11 ` Luiz Augusto von Dentz 0 siblings, 1 reply; 7+ messages in thread From: Sonny Sasaka @ 2020-12-11 23:31 UTC (permalink / raw) To: Luiz Augusto von Dentz; +Cc: linux-bluetooth@vger.kernel.org, Alain Michaud Hi Luiz, I sent a v2 that changes the cache to use gatt_db. Please take another look. Thanks! On Tue, Dec 8, 2020 at 5:58 PM Luiz Augusto von Dentz <luiz.dentz@gmail.com> wrote: > > Hi Sonny, > > On Tue, Dec 8, 2020 at 5:16 PM Sonny Sasaka <sonnysasaka@chromium.org> wrote: > > > > To optimize BLE HID devices reconnection response, we can cache the > > report map so that the subsequent reconnections do not need round trip > > time to read the report map. > > While the idea is pretty good we need to find a way to persist it > across reboots/restarts. In theory we could save the value on the > gatt_db instance and then make changes to device.c:store_desc to > attempt to read if there is a value stored in the db save it on file > as well so the next time the cache is loaded it should restore the > report map, or any other descriptor that have its value saved in the > database. That said we may need to introduce some sort of property for > values stored in the db since normally the values store in the > database shall not be persisted on file, in fact it would cause a lot > of fime access updating the cache if we were to trigger updates > everytime the db is updated with a value. > > > Reviewed-by: Alain Michaud <alainm@chromium.org> > > > > --- > > profiles/input/hog-lib.c | 142 ++++++++++++++++++++++++++------------- > > 1 file changed, 96 insertions(+), 46 deletions(-) > > > > diff --git a/profiles/input/hog-lib.c b/profiles/input/hog-lib.c > > index ee811d301..1e198ea64 100644 > > --- a/profiles/input/hog-lib.c > > +++ b/profiles/input/hog-lib.c > > @@ -95,6 +95,8 @@ struct bt_hog { > > struct queue *bas; > > GSList *instances; > > struct queue *gatt_op; > > + uint8_t report_map[HOG_REPORT_MAP_MAX_SIZE]; > > + ssize_t report_map_len; > > }; > > > > struct report { > > @@ -276,6 +278,8 @@ static void find_included(struct bt_hog *hog, GAttrib *attrib, > > free(req); > > } > > > > +static void uhid_create(struct bt_hog *hog); > > + > > static void report_value_cb(const guint8 *pdu, guint16 len, gpointer user_data) > > { > > struct report *report = user_data; > > @@ -924,57 +928,17 @@ static char *item2string(char *str, uint8_t *buf, uint8_t len) > > return str; > > } > > > > -static void report_map_read_cb(guint8 status, const guint8 *pdu, guint16 plen, > > - gpointer user_data) > > +static void uhid_create(struct bt_hog *hog) > > { > > - struct gatt_request *req = user_data; > > - struct bt_hog *hog = req->user_data; > > - uint8_t value[HOG_REPORT_MAP_MAX_SIZE]; > > struct uhid_event ev; > > - ssize_t vlen; > > - char itemstr[20]; /* 5x3 (data) + 4 (continuation) + 1 (null) */ > > - int i, err; > > GError *gerr = NULL; > > + int i, err; > > > > - destroy_gatt_req(req); > > - > > - DBG("HoG inspecting report map"); > > - > > - if (status != 0) { > > - error("Report Map read failed: %s", att_ecode2str(status)); > > - return; > > - } > > - > > - vlen = dec_read_resp(pdu, plen, value, sizeof(value)); > > - if (vlen < 0) { > > - error("ATT protocol error"); > > + if (!hog->report_map_len) { > > + warn("Failed to initiate UHID_CREATE without report map"); > > return; > > } > > > > - DBG("Report MAP:"); > > - for (i = 0; i < vlen;) { > > - ssize_t ilen = 0; > > - bool long_item = false; > > - > > - if (get_descriptor_item_info(&value[i], vlen - i, &ilen, > > - &long_item)) { > > - /* Report ID is short item with prefix 100001xx */ > > - if (!long_item && (value[i] & 0xfc) == 0x84) > > - hog->has_report_id = TRUE; > > - > > - DBG("\t%s", item2string(itemstr, &value[i], ilen)); > > - > > - i += ilen; > > - } else { > > - error("Report Map parsing failed at %d", i); > > - > > - /* Just print remaining items at once and break */ > > - DBG("\t%s", item2string(itemstr, &value[i], vlen - i)); > > - break; > > - } > > - } > > - > > - /* create uHID device */ > > memset(&ev, 0, sizeof(ev)); > > ev.type = UHID_CREATE; > > > > @@ -1004,8 +968,8 @@ static void report_map_read_cb(guint8 status, const guint8 *pdu, guint16 plen, > > ev.u.create.version = hog->version; > > ev.u.create.country = hog->bcountrycode; > > ev.u.create.bus = BUS_BLUETOOTH; > > - ev.u.create.rd_data = value; > > - ev.u.create.rd_size = vlen; > > + ev.u.create.rd_data = hog->report_map; > > + ev.u.create.rd_size = hog->report_map_len; > > > > err = bt_uhid_send(hog->uhid, &ev); > > if (err < 0) { > > @@ -1018,6 +982,62 @@ static void report_map_read_cb(guint8 status, const guint8 *pdu, guint16 plen, > > bt_uhid_register(hog->uhid, UHID_SET_REPORT, set_report, hog); > > > > hog->uhid_created = true; > > +} > > + > > +static void report_map_read_cb(guint8 status, const guint8 *pdu, guint16 plen, > > + gpointer user_data) > > +{ > > + struct gatt_request *req = user_data; > > + struct bt_hog *hog = req->user_data; > > + ssize_t vlen; > > + char itemstr[20]; /* 5x3 (data) + 4 (continuation) + 1 (null) */ > > + int i; > > + > > + destroy_gatt_req(req); > > + > > + DBG("HoG inspecting report map"); > > + > > + if (status != 0) { > > + error("Report Map read failed: %s", att_ecode2str(status)); > > + return; > > + } > > + > > + vlen = dec_read_resp(pdu, plen, hog->report_map, > > + sizeof(hog->report_map)); > > + if (vlen < 0) { > > + error("ATT protocol error"); > > + return; > > + } > > + > > + hog->report_map_len = vlen; > > + > > + DBG("Report MAP:"); > > + for (i = 0; i < vlen;) { > > + ssize_t ilen = 0; > > + bool long_item = false; > > + > > + if (get_descriptor_item_info(&hog->report_map[i], vlen - i, > > + &ilen, &long_item)) { > > + /* Report ID is short item with prefix 100001xx */ > > + if (!long_item && (hog->report_map[i] & 0xfc) == 0x84) > > + hog->has_report_id = TRUE; > > + > > + DBG("\t%s", item2string(itemstr, &hog->report_map[i], > > + ilen)); > > + > > + i += ilen; > > + } else { > > + error("Report Map parsing failed at %d", i); > > + > > + /* Just print remaining items at once and break */ > > + DBG("\t%s", item2string(itemstr, &hog->report_map[i], > > + vlen - i)); > > + break; > > + } > > + } > > + > > + /* create uHID device */ > > + uhid_create(hog); > > > > DBG("HoG created uHID device"); > > } > > @@ -1602,6 +1622,12 @@ bool bt_hog_attach(struct bt_hog *hog, void *gatt) > > bt_hog_attach(instance, gatt); > > } > > > > + /* Try to initiate UHID_CREATE if we already have the report map to > > + * avoid re-reading the report map from the peer device. > > + */ > > + if (hog->report_map_len > 0) > > + uhid_create(hog); > > + > > if (!hog->uhid_created) { > > DBG("HoG discovering characteristics"); > > if (hog->attr) > > @@ -1627,6 +1653,29 @@ bool bt_hog_attach(struct bt_hog *hog, void *gatt) > > return true; > > } > > > > +static void uhid_destroy(struct bt_hog *hog) > > +{ > > + int err; > > + struct uhid_event ev; > > + > > + if (!hog->uhid_created) > > + return; > > + > > + bt_uhid_unregister_all(hog->uhid); > > + > > + memset(&ev, 0, sizeof(ev)); > > + ev.type = UHID_DESTROY; > > + > > + err = bt_uhid_send(hog->uhid, &ev); > > + > > + if (err < 0) { > > + error("bt_uhid_send: %s", strerror(-err)); > > + return; > > + } > > + > > + hog->uhid_created = false; > > +} > > + > > void bt_hog_detach(struct bt_hog *hog) > > { > > GSList *l; > > @@ -1660,6 +1709,7 @@ void bt_hog_detach(struct bt_hog *hog) > > queue_foreach(hog->gatt_op, (void *) cancel_gatt_req, NULL); > > g_attrib_unref(hog->attrib); > > hog->attrib = NULL; > > + uhid_destroy(hog); > > } > > > > int bt_hog_set_control_point(struct bt_hog *hog, bool suspend) > > -- > > 2.26.2 > > > > > -- > Luiz Augusto von Dentz ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH BlueZ 2/2] input/hog: Cache the HID report map 2020-12-11 23:31 ` Sonny Sasaka @ 2020-12-12 1:11 ` Luiz Augusto von Dentz 2020-12-12 1:15 ` Sonny Sasaka 0 siblings, 1 reply; 7+ messages in thread From: Luiz Augusto von Dentz @ 2020-12-12 1:11 UTC (permalink / raw) To: Sonny Sasaka; +Cc: linux-bluetooth@vger.kernel.org, Alain Michaud Hi Sonny, On Fri, Dec 11, 2020 at 3:32 PM Sonny Sasaka <sonnysasaka@chromium.org> wrote: > > Hi Luiz, > > I sent a v2 that changes the cache to use gatt_db. Please take another > look. Thanks! It doesn't seem to have reached the list yet, or perhaps my mail server is playing tricks on me. > > On Tue, Dec 8, 2020 at 5:58 PM Luiz Augusto von Dentz > <luiz.dentz@gmail.com> wrote: > > > > Hi Sonny, > > > > On Tue, Dec 8, 2020 at 5:16 PM Sonny Sasaka <sonnysasaka@chromium.org> wrote: > > > > > > To optimize BLE HID devices reconnection response, we can cache the > > > report map so that the subsequent reconnections do not need round trip > > > time to read the report map. > > > > While the idea is pretty good we need to find a way to persist it > > across reboots/restarts. In theory we could save the value on the > > gatt_db instance and then make changes to device.c:store_desc to > > attempt to read if there is a value stored in the db save it on file > > as well so the next time the cache is loaded it should restore the > > report map, or any other descriptor that have its value saved in the > > database. That said we may need to introduce some sort of property for > > values stored in the db since normally the values store in the > > database shall not be persisted on file, in fact it would cause a lot > > of fime access updating the cache if we were to trigger updates > > everytime the db is updated with a value. > > > > > Reviewed-by: Alain Michaud <alainm@chromium.org> > > > > > > --- > > > profiles/input/hog-lib.c | 142 ++++++++++++++++++++++++++------------- > > > 1 file changed, 96 insertions(+), 46 deletions(-) > > > > > > diff --git a/profiles/input/hog-lib.c b/profiles/input/hog-lib.c > > > index ee811d301..1e198ea64 100644 > > > --- a/profiles/input/hog-lib.c > > > +++ b/profiles/input/hog-lib.c > > > @@ -95,6 +95,8 @@ struct bt_hog { > > > struct queue *bas; > > > GSList *instances; > > > struct queue *gatt_op; > > > + uint8_t report_map[HOG_REPORT_MAP_MAX_SIZE]; > > > + ssize_t report_map_len; > > > }; > > > > > > struct report { > > > @@ -276,6 +278,8 @@ static void find_included(struct bt_hog *hog, GAttrib *attrib, > > > free(req); > > > } > > > > > > +static void uhid_create(struct bt_hog *hog); > > > + > > > static void report_value_cb(const guint8 *pdu, guint16 len, gpointer user_data) > > > { > > > struct report *report = user_data; > > > @@ -924,57 +928,17 @@ static char *item2string(char *str, uint8_t *buf, uint8_t len) > > > return str; > > > } > > > > > > -static void report_map_read_cb(guint8 status, const guint8 *pdu, guint16 plen, > > > - gpointer user_data) > > > +static void uhid_create(struct bt_hog *hog) > > > { > > > - struct gatt_request *req = user_data; > > > - struct bt_hog *hog = req->user_data; > > > - uint8_t value[HOG_REPORT_MAP_MAX_SIZE]; > > > struct uhid_event ev; > > > - ssize_t vlen; > > > - char itemstr[20]; /* 5x3 (data) + 4 (continuation) + 1 (null) */ > > > - int i, err; > > > GError *gerr = NULL; > > > + int i, err; > > > > > > - destroy_gatt_req(req); > > > - > > > - DBG("HoG inspecting report map"); > > > - > > > - if (status != 0) { > > > - error("Report Map read failed: %s", att_ecode2str(status)); > > > - return; > > > - } > > > - > > > - vlen = dec_read_resp(pdu, plen, value, sizeof(value)); > > > - if (vlen < 0) { > > > - error("ATT protocol error"); > > > + if (!hog->report_map_len) { > > > + warn("Failed to initiate UHID_CREATE without report map"); > > > return; > > > } > > > > > > - DBG("Report MAP:"); > > > - for (i = 0; i < vlen;) { > > > - ssize_t ilen = 0; > > > - bool long_item = false; > > > - > > > - if (get_descriptor_item_info(&value[i], vlen - i, &ilen, > > > - &long_item)) { > > > - /* Report ID is short item with prefix 100001xx */ > > > - if (!long_item && (value[i] & 0xfc) == 0x84) > > > - hog->has_report_id = TRUE; > > > - > > > - DBG("\t%s", item2string(itemstr, &value[i], ilen)); > > > - > > > - i += ilen; > > > - } else { > > > - error("Report Map parsing failed at %d", i); > > > - > > > - /* Just print remaining items at once and break */ > > > - DBG("\t%s", item2string(itemstr, &value[i], vlen - i)); > > > - break; > > > - } > > > - } > > > - > > > - /* create uHID device */ > > > memset(&ev, 0, sizeof(ev)); > > > ev.type = UHID_CREATE; > > > > > > @@ -1004,8 +968,8 @@ static void report_map_read_cb(guint8 status, const guint8 *pdu, guint16 plen, > > > ev.u.create.version = hog->version; > > > ev.u.create.country = hog->bcountrycode; > > > ev.u.create.bus = BUS_BLUETOOTH; > > > - ev.u.create.rd_data = value; > > > - ev.u.create.rd_size = vlen; > > > + ev.u.create.rd_data = hog->report_map; > > > + ev.u.create.rd_size = hog->report_map_len; > > > > > > err = bt_uhid_send(hog->uhid, &ev); > > > if (err < 0) { > > > @@ -1018,6 +982,62 @@ static void report_map_read_cb(guint8 status, const guint8 *pdu, guint16 plen, > > > bt_uhid_register(hog->uhid, UHID_SET_REPORT, set_report, hog); > > > > > > hog->uhid_created = true; > > > +} > > > + > > > +static void report_map_read_cb(guint8 status, const guint8 *pdu, guint16 plen, > > > + gpointer user_data) > > > +{ > > > + struct gatt_request *req = user_data; > > > + struct bt_hog *hog = req->user_data; > > > + ssize_t vlen; > > > + char itemstr[20]; /* 5x3 (data) + 4 (continuation) + 1 (null) */ > > > + int i; > > > + > > > + destroy_gatt_req(req); > > > + > > > + DBG("HoG inspecting report map"); > > > + > > > + if (status != 0) { > > > + error("Report Map read failed: %s", att_ecode2str(status)); > > > + return; > > > + } > > > + > > > + vlen = dec_read_resp(pdu, plen, hog->report_map, > > > + sizeof(hog->report_map)); > > > + if (vlen < 0) { > > > + error("ATT protocol error"); > > > + return; > > > + } > > > + > > > + hog->report_map_len = vlen; > > > + > > > + DBG("Report MAP:"); > > > + for (i = 0; i < vlen;) { > > > + ssize_t ilen = 0; > > > + bool long_item = false; > > > + > > > + if (get_descriptor_item_info(&hog->report_map[i], vlen - i, > > > + &ilen, &long_item)) { > > > + /* Report ID is short item with prefix 100001xx */ > > > + if (!long_item && (hog->report_map[i] & 0xfc) == 0x84) > > > + hog->has_report_id = TRUE; > > > + > > > + DBG("\t%s", item2string(itemstr, &hog->report_map[i], > > > + ilen)); > > > + > > > + i += ilen; > > > + } else { > > > + error("Report Map parsing failed at %d", i); > > > + > > > + /* Just print remaining items at once and break */ > > > + DBG("\t%s", item2string(itemstr, &hog->report_map[i], > > > + vlen - i)); > > > + break; > > > + } > > > + } > > > + > > > + /* create uHID device */ > > > + uhid_create(hog); > > > > > > DBG("HoG created uHID device"); > > > } > > > @@ -1602,6 +1622,12 @@ bool bt_hog_attach(struct bt_hog *hog, void *gatt) > > > bt_hog_attach(instance, gatt); > > > } > > > > > > + /* Try to initiate UHID_CREATE if we already have the report map to > > > + * avoid re-reading the report map from the peer device. > > > + */ > > > + if (hog->report_map_len > 0) > > > + uhid_create(hog); > > > + > > > if (!hog->uhid_created) { > > > DBG("HoG discovering characteristics"); > > > if (hog->attr) > > > @@ -1627,6 +1653,29 @@ bool bt_hog_attach(struct bt_hog *hog, void *gatt) > > > return true; > > > } > > > > > > +static void uhid_destroy(struct bt_hog *hog) > > > +{ > > > + int err; > > > + struct uhid_event ev; > > > + > > > + if (!hog->uhid_created) > > > + return; > > > + > > > + bt_uhid_unregister_all(hog->uhid); > > > + > > > + memset(&ev, 0, sizeof(ev)); > > > + ev.type = UHID_DESTROY; > > > + > > > + err = bt_uhid_send(hog->uhid, &ev); > > > + > > > + if (err < 0) { > > > + error("bt_uhid_send: %s", strerror(-err)); > > > + return; > > > + } > > > + > > > + hog->uhid_created = false; > > > +} > > > + > > > void bt_hog_detach(struct bt_hog *hog) > > > { > > > GSList *l; > > > @@ -1660,6 +1709,7 @@ void bt_hog_detach(struct bt_hog *hog) > > > queue_foreach(hog->gatt_op, (void *) cancel_gatt_req, NULL); > > > g_attrib_unref(hog->attrib); > > > hog->attrib = NULL; > > > + uhid_destroy(hog); > > > } > > > > > > int bt_hog_set_control_point(struct bt_hog *hog, bool suspend) > > > -- > > > 2.26.2 > > > > > > > > > -- > > Luiz Augusto von Dentz -- Luiz Augusto von Dentz ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH BlueZ 2/2] input/hog: Cache the HID report map 2020-12-12 1:11 ` Luiz Augusto von Dentz @ 2020-12-12 1:15 ` Sonny Sasaka 0 siblings, 0 replies; 7+ messages in thread From: Sonny Sasaka @ 2020-12-12 1:15 UTC (permalink / raw) To: Luiz Augusto von Dentz; +Cc: linux-bluetooth@vger.kernel.org, Alain Michaud Hi Luiz, It's already here: https://patchwork.kernel.org/project/bluetooth/patch/20201211233047.108658-2-sonnysasaka@chromium.org/. On Fri, Dec 11, 2020 at 5:12 PM Luiz Augusto von Dentz <luiz.dentz@gmail.com> wrote: > > Hi Sonny, > > On Fri, Dec 11, 2020 at 3:32 PM Sonny Sasaka <sonnysasaka@chromium.org> wrote: > > > > Hi Luiz, > > > > I sent a v2 that changes the cache to use gatt_db. Please take another > > look. Thanks! > > It doesn't seem to have reached the list yet, or perhaps my mail > server is playing tricks on me. > > > > > On Tue, Dec 8, 2020 at 5:58 PM Luiz Augusto von Dentz > > <luiz.dentz@gmail.com> wrote: > > > > > > Hi Sonny, > > > > > > On Tue, Dec 8, 2020 at 5:16 PM Sonny Sasaka <sonnysasaka@chromium.org> wrote: > > > > > > > > To optimize BLE HID devices reconnection response, we can cache the > > > > report map so that the subsequent reconnections do not need round trip > > > > time to read the report map. > > > > > > While the idea is pretty good we need to find a way to persist it > > > across reboots/restarts. In theory we could save the value on the > > > gatt_db instance and then make changes to device.c:store_desc to > > > attempt to read if there is a value stored in the db save it on file > > > as well so the next time the cache is loaded it should restore the > > > report map, or any other descriptor that have its value saved in the > > > database. That said we may need to introduce some sort of property for > > > values stored in the db since normally the values store in the > > > database shall not be persisted on file, in fact it would cause a lot > > > of fime access updating the cache if we were to trigger updates > > > everytime the db is updated with a value. > > > > > > > Reviewed-by: Alain Michaud <alainm@chromium.org> > > > > > > > > --- > > > > profiles/input/hog-lib.c | 142 ++++++++++++++++++++++++++------------- > > > > 1 file changed, 96 insertions(+), 46 deletions(-) > > > > > > > > diff --git a/profiles/input/hog-lib.c b/profiles/input/hog-lib.c > > > > index ee811d301..1e198ea64 100644 > > > > --- a/profiles/input/hog-lib.c > > > > +++ b/profiles/input/hog-lib.c > > > > @@ -95,6 +95,8 @@ struct bt_hog { > > > > struct queue *bas; > > > > GSList *instances; > > > > struct queue *gatt_op; > > > > + uint8_t report_map[HOG_REPORT_MAP_MAX_SIZE]; > > > > + ssize_t report_map_len; > > > > }; > > > > > > > > struct report { > > > > @@ -276,6 +278,8 @@ static void find_included(struct bt_hog *hog, GAttrib *attrib, > > > > free(req); > > > > } > > > > > > > > +static void uhid_create(struct bt_hog *hog); > > > > + > > > > static void report_value_cb(const guint8 *pdu, guint16 len, gpointer user_data) > > > > { > > > > struct report *report = user_data; > > > > @@ -924,57 +928,17 @@ static char *item2string(char *str, uint8_t *buf, uint8_t len) > > > > return str; > > > > } > > > > > > > > -static void report_map_read_cb(guint8 status, const guint8 *pdu, guint16 plen, > > > > - gpointer user_data) > > > > +static void uhid_create(struct bt_hog *hog) > > > > { > > > > - struct gatt_request *req = user_data; > > > > - struct bt_hog *hog = req->user_data; > > > > - uint8_t value[HOG_REPORT_MAP_MAX_SIZE]; > > > > struct uhid_event ev; > > > > - ssize_t vlen; > > > > - char itemstr[20]; /* 5x3 (data) + 4 (continuation) + 1 (null) */ > > > > - int i, err; > > > > GError *gerr = NULL; > > > > + int i, err; > > > > > > > > - destroy_gatt_req(req); > > > > - > > > > - DBG("HoG inspecting report map"); > > > > - > > > > - if (status != 0) { > > > > - error("Report Map read failed: %s", att_ecode2str(status)); > > > > - return; > > > > - } > > > > - > > > > - vlen = dec_read_resp(pdu, plen, value, sizeof(value)); > > > > - if (vlen < 0) { > > > > - error("ATT protocol error"); > > > > + if (!hog->report_map_len) { > > > > + warn("Failed to initiate UHID_CREATE without report map"); > > > > return; > > > > } > > > > > > > > - DBG("Report MAP:"); > > > > - for (i = 0; i < vlen;) { > > > > - ssize_t ilen = 0; > > > > - bool long_item = false; > > > > - > > > > - if (get_descriptor_item_info(&value[i], vlen - i, &ilen, > > > > - &long_item)) { > > > > - /* Report ID is short item with prefix 100001xx */ > > > > - if (!long_item && (value[i] & 0xfc) == 0x84) > > > > - hog->has_report_id = TRUE; > > > > - > > > > - DBG("\t%s", item2string(itemstr, &value[i], ilen)); > > > > - > > > > - i += ilen; > > > > - } else { > > > > - error("Report Map parsing failed at %d", i); > > > > - > > > > - /* Just print remaining items at once and break */ > > > > - DBG("\t%s", item2string(itemstr, &value[i], vlen - i)); > > > > - break; > > > > - } > > > > - } > > > > - > > > > - /* create uHID device */ > > > > memset(&ev, 0, sizeof(ev)); > > > > ev.type = UHID_CREATE; > > > > > > > > @@ -1004,8 +968,8 @@ static void report_map_read_cb(guint8 status, const guint8 *pdu, guint16 plen, > > > > ev.u.create.version = hog->version; > > > > ev.u.create.country = hog->bcountrycode; > > > > ev.u.create.bus = BUS_BLUETOOTH; > > > > - ev.u.create.rd_data = value; > > > > - ev.u.create.rd_size = vlen; > > > > + ev.u.create.rd_data = hog->report_map; > > > > + ev.u.create.rd_size = hog->report_map_len; > > > > > > > > err = bt_uhid_send(hog->uhid, &ev); > > > > if (err < 0) { > > > > @@ -1018,6 +982,62 @@ static void report_map_read_cb(guint8 status, const guint8 *pdu, guint16 plen, > > > > bt_uhid_register(hog->uhid, UHID_SET_REPORT, set_report, hog); > > > > > > > > hog->uhid_created = true; > > > > +} > > > > + > > > > +static void report_map_read_cb(guint8 status, const guint8 *pdu, guint16 plen, > > > > + gpointer user_data) > > > > +{ > > > > + struct gatt_request *req = user_data; > > > > + struct bt_hog *hog = req->user_data; > > > > + ssize_t vlen; > > > > + char itemstr[20]; /* 5x3 (data) + 4 (continuation) + 1 (null) */ > > > > + int i; > > > > + > > > > + destroy_gatt_req(req); > > > > + > > > > + DBG("HoG inspecting report map"); > > > > + > > > > + if (status != 0) { > > > > + error("Report Map read failed: %s", att_ecode2str(status)); > > > > + return; > > > > + } > > > > + > > > > + vlen = dec_read_resp(pdu, plen, hog->report_map, > > > > + sizeof(hog->report_map)); > > > > + if (vlen < 0) { > > > > + error("ATT protocol error"); > > > > + return; > > > > + } > > > > + > > > > + hog->report_map_len = vlen; > > > > + > > > > + DBG("Report MAP:"); > > > > + for (i = 0; i < vlen;) { > > > > + ssize_t ilen = 0; > > > > + bool long_item = false; > > > > + > > > > + if (get_descriptor_item_info(&hog->report_map[i], vlen - i, > > > > + &ilen, &long_item)) { > > > > + /* Report ID is short item with prefix 100001xx */ > > > > + if (!long_item && (hog->report_map[i] & 0xfc) == 0x84) > > > > + hog->has_report_id = TRUE; > > > > + > > > > + DBG("\t%s", item2string(itemstr, &hog->report_map[i], > > > > + ilen)); > > > > + > > > > + i += ilen; > > > > + } else { > > > > + error("Report Map parsing failed at %d", i); > > > > + > > > > + /* Just print remaining items at once and break */ > > > > + DBG("\t%s", item2string(itemstr, &hog->report_map[i], > > > > + vlen - i)); > > > > + break; > > > > + } > > > > + } > > > > + > > > > + /* create uHID device */ > > > > + uhid_create(hog); > > > > > > > > DBG("HoG created uHID device"); > > > > } > > > > @@ -1602,6 +1622,12 @@ bool bt_hog_attach(struct bt_hog *hog, void *gatt) > > > > bt_hog_attach(instance, gatt); > > > > } > > > > > > > > + /* Try to initiate UHID_CREATE if we already have the report map to > > > > + * avoid re-reading the report map from the peer device. > > > > + */ > > > > + if (hog->report_map_len > 0) > > > > + uhid_create(hog); > > > > + > > > > if (!hog->uhid_created) { > > > > DBG("HoG discovering characteristics"); > > > > if (hog->attr) > > > > @@ -1627,6 +1653,29 @@ bool bt_hog_attach(struct bt_hog *hog, void *gatt) > > > > return true; > > > > } > > > > > > > > +static void uhid_destroy(struct bt_hog *hog) > > > > +{ > > > > + int err; > > > > + struct uhid_event ev; > > > > + > > > > + if (!hog->uhid_created) > > > > + return; > > > > + > > > > + bt_uhid_unregister_all(hog->uhid); > > > > + > > > > + memset(&ev, 0, sizeof(ev)); > > > > + ev.type = UHID_DESTROY; > > > > + > > > > + err = bt_uhid_send(hog->uhid, &ev); > > > > + > > > > + if (err < 0) { > > > > + error("bt_uhid_send: %s", strerror(-err)); > > > > + return; > > > > + } > > > > + > > > > + hog->uhid_created = false; > > > > +} > > > > + > > > > void bt_hog_detach(struct bt_hog *hog) > > > > { > > > > GSList *l; > > > > @@ -1660,6 +1709,7 @@ void bt_hog_detach(struct bt_hog *hog) > > > > queue_foreach(hog->gatt_op, (void *) cancel_gatt_req, NULL); > > > > g_attrib_unref(hog->attrib); > > > > hog->attrib = NULL; > > > > + uhid_destroy(hog); > > > > } > > > > > > > > int bt_hog_set_control_point(struct bt_hog *hog, bool suspend) > > > > -- > > > > 2.26.2 > > > > > > > > > > > > > -- > > > Luiz Augusto von Dentz > > > > -- > Luiz Augusto von Dentz ^ permalink raw reply [flat|nested] 7+ messages in thread
* RE: [BlueZ,1/2] Revert "input/hog: Remove HID device after HoG device disconnects" 2020-12-09 1:00 [PATCH BlueZ 1/2] Revert "input/hog: Remove HID device after HoG device disconnects" Sonny Sasaka 2020-12-09 1:00 ` [PATCH BlueZ 2/2] input/hog: Cache the HID report map Sonny Sasaka @ 2020-12-09 1:56 ` bluez.test.bot 1 sibling, 0 replies; 7+ messages in thread From: bluez.test.bot @ 2020-12-09 1:56 UTC (permalink / raw) To: linux-bluetooth, sonnysasaka [-- Attachment #1: Type: text/plain, Size: 557 bytes --] This is automated email and please do not reply to this email! Dear submitter, Thank you for submitting the patches to the linux bluetooth mailing list. This is a CI test results with your patch series: PW Link:https://patchwork.kernel.org/project/bluetooth/list/?series=398575 ---Test result--- ############################## Test: CheckPatch - PASS ############################## Test: CheckGitLint - PASS ############################## Test: CheckBuild - PASS ############################## Test: MakeCheck - PASS --- Regards, Linux Bluetooth ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2020-12-12 1:18 UTC | newest] Thread overview: 7+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2020-12-09 1:00 [PATCH BlueZ 1/2] Revert "input/hog: Remove HID device after HoG device disconnects" Sonny Sasaka 2020-12-09 1:00 ` [PATCH BlueZ 2/2] input/hog: Cache the HID report map Sonny Sasaka 2020-12-09 1:57 ` Luiz Augusto von Dentz 2020-12-11 23:31 ` Sonny Sasaka 2020-12-12 1:11 ` Luiz Augusto von Dentz 2020-12-12 1:15 ` Sonny Sasaka 2020-12-09 1:56 ` [BlueZ,1/2] Revert "input/hog: Remove HID device after HoG device disconnects" bluez.test.bot
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).