Re: [PR] Blestress fixes [mynewt-nimble]
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]
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]
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]
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]
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]
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]
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]
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]
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