[GitHub] ccollins476ad commented on a change in pull request #293: apps/btshell: add bletiny clone that uses new shell
ccollins476ad commented on a change in pull request #293: apps/btshell: add
bletiny clone that uses new shell
URL:
https://github.com/apache/incubator-mynewt-core/pull/293#discussion_r121025656
##
File path: net/nimble/host/src/ble_gatts.c
##
@@ -2080,6 +2080,141 @@ ble_gatts_count_cfg(const struct ble_gatt_svc_def
*defs)
return 0;
}
+static const char* ble_gatt_chr_f_names[] = {
+"BROADCAST",
+"READ",
+"WRITE_NO_RSP",
+"WRITE",
+"NOTIFY",
+"INDICATE",
+"AUTH_SIGN_WRITE",
+"RELIABLE_WRITE",
+"AUX_WRITE",
+"READ_ENC",
+"READ_AUTHEN",
+"READ_AUTHOR",
+"WRITE_ENC",
+"WRITE_AUTHEN",
+"WRITE_AUTHOR",
+NULL
+};
+
+static const char* ble_gatt_dsc_f_names[] = {
+"READ",
+"WRITE",
+"READ_ENC",
+"READ_AUTHEN",
+"READ_AUTHOR",
+"WRITE_ENC",
+"WRITE_AUTHEN",
+"WRITE_AUTHOR",
+NULL
+};
+
+#define BLE_CHR_FLAGS_STR_LEN 180
+
+static char *
+ble_gatts_flags_to_str(uint16_t flags, char *buf,
+ const char **names)
+{
+int bit;
+bool non_empty = false;
+size_t length = 0;
+
+buf[0] = '\0';
+strcpy(buf, "[");
+length += 1;
+for (bit = 0; names[bit]; ++bit) {
+if (flags & (1 << bit)) {
+length += strlen(names[bit]);
+if (length + 1 >= BLE_CHR_FLAGS_STR_LEN) {
+return buf;
+}
+if (non_empty) {
+strcat(buf, "|");
+length += 1;
+}
+strcat(buf, names[bit]);
+non_empty = true;
+}
+}
+strcat(buf, "]");
+return buf;
+}
+
+
+#define STRINGIFY(X) #X
+#define FIELD_NAME_LEN STRINGIFY(12)
+#define FIELD_INDENT STRINGIFY(2)
+
+static void
+ble_gatt_show_local_chr(const struct ble_gatt_svc_def *svc,
Review comment:
This may verge on nit-picking, but I am not sure about putting code to print
the registered resources in this file (ble_gatts.c). This file is already
pretty big and ugly, and this new functionality feels different from what is
already there.
I would suggest one of the following:
1.
* Move ble_gatt_show_local_chr() to a new file.
* Create ble_gatt_show_local_svc() in the new file.
* Modify ble_gatts_show_local() such that it just iterates the list of
services, calling ble_gatt_show_local_svc() on each one.
This removes most of the new code from ble_gatts.c
2. Add accessor functions for the service list; move all new code into a new
file. ble_gatts_show_local() calls the new accessor function and prints each
service.
3. Add a generic ble_gatts_lcl_svc_foreach() function to ble_gatts.c. This
function takes a function pointer which gets called for each registered
service. The new print code could go in a new file and be implemented with a
call to the new foreach function.
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] ccollins476ad commented on a change in pull request #293: apps/btshell: add bletiny clone that uses new shell
ccollins476ad commented on a change in pull request #293: apps/btshell: add
bletiny clone that uses new shell
URL:
https://github.com/apache/incubator-mynewt-core/pull/293#discussion_r121022904
##
File path: apps/btshell/src/cmd.c
##
@@ -0,0 +1,2515 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+#include "syscfg/syscfg.h"
+
+#include
+#include
+#include
+#include
+#include "bsp/bsp.h"
+
+#include "nimble/ble.h"
+#include "nimble/nimble_opt.h"
+#include "nimble/hci_common.h"
+#include "host/ble_gap.h"
+#include "host/ble_hs_adv.h"
+#include "host/ble_sm.h"
+#include "host/ble_eddystone.h"
+#include "host/ble_hs_id.h"
+#include "services/gatt/ble_svc_gatt.h"
+#include "../src/ble_hs_priv.h"
+
+#include "console/console.h"
+#include "shell/shell.h"
+
+#include "cmd.h"
+#include "bletiny.h"
+#include "cmd_gatt.h"
+#include "cmd_l2cap.h"
+
+#define BTSHELL_MODULE "btshell"
+
+
+int
+cmd_parse_conn_start_end(uint16_t *out_conn, uint16_t *out_start,
+ uint16_t *out_end)
+{
+int rc;
+
+*out_conn = parse_arg_uint16("conn", );
+if (rc != 0) {
+return rc;
+}
+
+*out_start = parse_arg_uint16("start", );
+if (rc != 0) {
+return rc;
+}
+
+*out_end = parse_arg_uint16("end", );
+if (rc != 0) {
+return rc;
+}
+
+return 0;
+}
+
+static struct kv_pair cmd_own_addr_types[] = {
Review comment:
These arrays should all be const so that they can be placed in .text (yes, I
know I am the one who screwed this up in bletiny!)
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] ccollins476ad commented on a change in pull request #293: apps/btshell: add bletiny clone that uses new shell
ccollins476ad commented on a change in pull request #293: apps/btshell: add bletiny clone that uses new shell URL: https://github.com/apache/incubator-mynewt-core/pull/293#discussion_r121022811 ## File path: apps/btshell/src/parse.c ## @@ -0,0 +1,611 @@ +/* Review comment: Btw, a lot of this file (parse.c) has been extracted from bletiny into a new package: util/parse. I wouldn't expect it to be part of this pull request, but at some point we should think about leveraging that package and removing the duplicate code in these apps. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] ccollins476ad commented on a change in pull request #293: apps/btshell: add bletiny clone that uses new shell
ccollins476ad commented on a change in pull request #293: apps/btshell: add
bletiny clone that uses new shell
URL:
https://github.com/apache/incubator-mynewt-core/pull/293#discussion_r121020741
##
File path: net/nimble/host/src/ble_gatts.c
##
@@ -2080,6 +2080,141 @@ ble_gatts_count_cfg(const struct ble_gatt_svc_def
*defs)
return 0;
}
+static const char* ble_gatt_chr_f_names[] = {
Review comment:
The pointed-to strings are const, but the pointers themselves aren't. The
pointers should probably be made const so that this array can be placed in
.text rather than .data.
E.g.,
`static const char * const ble_gatt_chr_f_names[]`
The same comment applies to the ble_gatt_dsc_f_names array below.
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
