Re: [PR] Blestress fixes [mynewt-nimble]

2024-03-03 Thread via GitHub


KKopyscinski merged PR #986:
URL: https://github.com/apache/mynewt-nimble/pull/986


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscr...@mynewt.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org



Re: [PR] Blestress fixes [mynewt-nimble]

2024-02-11 Thread via GitHub


KKopyscinski commented on code in PR #986:
URL: https://github.com/apache/mynewt-nimble/pull/986#discussion_r1485819268


##
apps/blestress/src/rx_stress.c:
##
@@ -1444,14 +1444,11 @@ rx_stress_main_task_fn(void *arg)
 os_sem_pend(_stress_main_sem, OS_TIMEOUT_NEVER);
 
 /* Standard tests perform */
-for (i = 11; i < STRESS_UUIDS_NUM; ++i) {
-if (i == 7 || i == 8 || i == 13) {
-/* 7,8: PHY update tests cause that the device during the next test
- * will stuck somewhere and will reset. Skip them for now.
- * 13: Should work after fixing ble_gattc_notify_custom (nimble 
issue on GitHub)*/
+for (i = 2; i < STRESS_UUIDS_NUM; ++i) {

Review Comment:
   Yes, but modified - looks like all tests are working now and neither should 
be skipped.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscr...@mynewt.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org



Re: [PR] Blestress fixes [mynewt-nimble]

2024-02-11 Thread via GitHub


KKopyscinski commented on code in PR #986:
URL: https://github.com/apache/mynewt-nimble/pull/986#discussion_r1485812777


##
apps/blestress/src/tx_stress.c:
##
@@ -848,6 +848,16 @@ tx_stress_9_gap_event(struct ble_gap_event *event, void 
*arg)
 {
 ble_addr_t addr;
 int test;
+struct ble_gap_conn_params conn_params = {
+.scan_itvl = 0x0010,
+.scan_window = 0x0010,
+.itvl_min = BLE_GAP_INITIAL_CONN_ITVL_MIN,
+.itvl_max = BLE_GAP_INITIAL_CONN_ITVL_MAX,
+.latency = 0,
+.supervision_timeout = 0x0C80,
+.min_ce_len = 0x0010,
+.max_ce_len = 0x0300,
+};

Review Comment:
   There are total of 15 successful concurrent connections. For example run, 
last succesfull connection occurs ~164343728us timestamp and first disconnect 
at 196078120us. This gives us 31.734392s. 0x0C80 is 3200, x10ms = 32s, which 
considering print delays seems pretty close IMO.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscr...@mynewt.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org



Re: [PR] Blestress fixes [mynewt-nimble]

2024-02-11 Thread via GitHub


KKopyscinski commented on code in PR #986:
URL: https://github.com/apache/mynewt-nimble/pull/986#discussion_r1485804379


##
apps/blestress/src/stress.h:
##
@@ -39,7 +39,7 @@ extern "C" {
 #define TX_PHY_MASK 0
 #define RX_PHY_MASK 0
 /* L2CAP SDU */
-#define STRESS_COC_MTU (64000)

Review Comment:
   I'm not sure what was wrong there, reading commit message I mentioned some 
issue with `ble_l2cap_coc_rx_fn`. Whatever this was, was fixed and this commit 
is not needed - I retested it. I suspect that 
https://github.com/apache/mynewt-nimble/commit/c1a2c9949805148520afb852d86afe588061d64e
 was the fix



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscr...@mynewt.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org



Re: [PR] Blestress fixes [mynewt-nimble]

2024-02-11 Thread via GitHub


KKopyscinski commented on code in PR #986:
URL: https://github.com/apache/mynewt-nimble/pull/986#discussion_r1485787601


##
apps/blestress/syscfg.yml:
##
@@ -75,3 +75,6 @@ syscfg.vals:
 
 # Whether to save data to sys/config, or just keep it in RAM.
 BLE_STORE_CONFIG_PERSIST: 0
+
+# ACL buf size must be able to contain CoC MPS plus ACL header
+BLE_ACL_BUF_SIZE: MYNEWT_VAL_BLE_L2CAP_COC_MPS+4

Review Comment:
   This incorrectly expands to
   ``` c
   #define MYNEWT_VAL_BLE_TRANSPORT_ACL_SIZE (MYNEWT_VAL_MSYS_1_BLOCK_SIZE-8)
   ```
   and ignores +4. So we need to use `MYNEWT_VAL_` variant. I think it's the 
same as for `BLE_L2CAP_COC_MPS` definition in `nimble/host/syscfg.yml`



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscr...@mynewt.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org



Re: [PR] Blestress fixes [mynewt-nimble]

2024-02-11 Thread via GitHub


rymanluk commented on code in PR #986:
URL: https://github.com/apache/mynewt-nimble/pull/986#discussion_r1485630074


##
apps/blestress/src/rx_stress.c:
##
@@ -1444,14 +1444,11 @@ rx_stress_main_task_fn(void *arg)
 os_sem_pend(_stress_main_sem, OS_TIMEOUT_NEVER);
 
 /* Standard tests perform */
-for (i = 11; i < STRESS_UUIDS_NUM; ++i) {
-if (i == 7 || i == 8 || i == 13) {
-/* 7,8: PHY update tests cause that the device during the next test
- * will stuck somewhere and will reset. Skip them for now.
- * 13: Should work after fixing ble_gattc_notify_custom (nimble 
issue on GitHub)*/
+for (i = 2; i < STRESS_UUIDS_NUM; ++i) {

Review Comment:
   is this patch still needeD?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscr...@mynewt.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org



Re: [PR] Blestress fixes [mynewt-nimble]

2024-02-11 Thread via GitHub


rymanluk commented on code in PR #986:
URL: https://github.com/apache/mynewt-nimble/pull/986#discussion_r1485629884


##
apps/blestress/src/tx_stress.c:
##
@@ -848,6 +848,16 @@ tx_stress_9_gap_event(struct ble_gap_event *event, void 
*arg)
 {
 ble_addr_t addr;
 int test;
+struct ble_gap_conn_params conn_params = {
+.scan_itvl = 0x0010,
+.scan_window = 0x0010,
+.itvl_min = BLE_GAP_INITIAL_CONN_ITVL_MIN,
+.itvl_max = BLE_GAP_INITIAL_CONN_ITVL_MAX,
+.latency = 0,
+.supervision_timeout = 0x0C80,
+.min_ce_len = 0x0010,
+.max_ce_len = 0x0300,
+};

Review Comment:
   @KKopyscinski ?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscr...@mynewt.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org



Re: [PR] Blestress fixes [mynewt-nimble]

2024-02-11 Thread via GitHub


rymanluk commented on code in PR #986:
URL: https://github.com/apache/mynewt-nimble/pull/986#discussion_r1485629555


##
apps/blestress/src/stress.h:
##
@@ -39,7 +39,7 @@ extern "C" {
 #define TX_PHY_MASK 0
 #define RX_PHY_MASK 0
 /* L2CAP SDU */
-#define STRESS_COC_MTU (64000)

Review Comment:
   why we need to decrease it ?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscr...@mynewt.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org



Re: [PR] Blestress fixes [mynewt-nimble]

2024-02-11 Thread via GitHub


rymanluk commented on code in PR #986:
URL: https://github.com/apache/mynewt-nimble/pull/986#discussion_r1485628446


##
apps/blestress/syscfg.yml:
##
@@ -75,3 +75,6 @@ syscfg.vals:
 
 # Whether to save data to sys/config, or just keep it in RAM.
 BLE_STORE_CONFIG_PERSIST: 0
+
+# ACL buf size must be able to contain CoC MPS plus ACL header
+BLE_ACL_BUF_SIZE: MYNEWT_VAL_BLE_L2CAP_COC_MPS+4

Review Comment:
   shouldn't it be (MYNEWT_VAL(MYNEWT_VAL_BLE_L2CAP_COC_MPS) + 4 ) ?



##
apps/blestress/syscfg.yml:
##
@@ -75,3 +75,6 @@ syscfg.vals:
 
 # Whether to save data to sys/config, or just keep it in RAM.
 BLE_STORE_CONFIG_PERSIST: 0
+
+# ACL buf size must be able to contain CoC MPS plus ACL header
+BLE_ACL_BUF_SIZE: MYNEWT_VAL_BLE_L2CAP_COC_MPS+4

Review Comment:
   shouldn't it be (MYNEWT_VAL(BLE_L2CAP_COC_MPS) + 4 ) ?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscr...@mynewt.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org