Blestress fixes #986
Blestress fixes #986
Conversation
KKopyscinski
force-pushed
the
blestress-fixes
branch
from
cc1b63b
to
09cadd3
Compare
|
@mkasenberg please review these changes |
|
There is still crash in test 14 - I'll update a PR with fix |
KKopyscinski
force-pushed
the
blestress-fixes
branch
from
6060e70
to
b5ece90
Compare
|
All crashes are resolved - the only problem left is silent restart on RX side during test 9 if test 7 was executed before - for now, skip test 7. Reason of this restart is yet unknown - there is no crash visible in debugger, no crash information is printed to console, HCI trace only shows lack of activity - basically, restart causes RTT to deattach. All tests are working if run independently. |
| .supervision_timeout = 0x0C80, | ||
| .min_ce_len = 0x0010, | ||
| .max_ce_len = 0x0300, | ||
| }; |
There was a problem hiding this comment.
are those connections maintained in test for at least this timeout after last one is created?
There was a problem hiding this 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.
KKopyscinski
force-pushed
the
blestress-fixes
branch
from
b5ece90
to
385ec20
Compare
|
please rebase |
apps/blestress/syscfg.yml
Outdated
| @@ -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 | |||
There was a problem hiding this comment.
shouldn't it be (MYNEWT_VAL(BLE_L2CAP_COC_MPS) + 4 ) ?
There was a problem hiding this comment.
This incorrectly expands to
#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
apps/blestress/src/stress.h
Outdated
| @@ -39,7 +39,7 @@ extern "C" { | |||
| #define TX_PHY_MASK 0 | |||
| #define RX_PHY_MASK 0 | |||
| /* L2CAP SDU */ | |||
| #define STRESS_COC_MTU (64000) | |||
There was a problem hiding this comment.
why we need to decrease it ?
There was a problem hiding this 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 c1a2c99 was the fix
apps/blestress/src/rx_stress.c
Outdated
| /* 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) { |
There was a problem hiding this comment.
is this patch still needeD?
There was a problem hiding this comment.
Yes, but modified - looks like all tests are working now and neither should be skipped.
Disable filtering of advertising reports, as we check the same data N times, and only first time is registered with filtering enabled.
With current (default) CoC MPS setting we were trying to send more than HCI packet could hold. This patch increases its size to sufficient value.
By adding connection params with increased supervision timeout we are able to make more connections before previous are closed. Supervision timeout is set to its maximum value of 32s (0x0C80 in 10ms units).This allows to achieve number of connections declared to be supported by NimBLE: 32+.
All implemented tests are working now - run all of them.
KKopyscinski
force-pushed
the
blestress-fixes
branch
from
385ec20
to
60ff84d
Compare
No description provided.