Hi Zhengping, On Wed, Jan 25, 2023 at 11:16 PM Zhengping Jiang <jiangzp@xxxxxxxxxx> wrote: > > Hi Luiz, > > > What test are you talking about? The one in mgmt-tester which the CI > runs or your own tests? > > Sorry for the confusion, I mean our own tests. Are the advertisement > instances fully cleared before the hci_sync rework? I looked at the > old hci_power_off before commit cf75ad8b41d2aa0 and did not find the > code to do that. The test expects Advertising Removed (0x0024) to pass: https://gist.github.com/Vudentz/82938a4b7d501d5fea43a3b71e265e12 Like I said I suspect that test was broken, perhaps we change the behavior but did not change the tests which then caused us to revert to the old behavior in order to pass the test, anyway Add Advertising does allow instances to be programmed while powered off but Add Extended Advertising Data doesn't: https://git.kernel.org/pub/scm/bluetooth/bluez.git/tree/doc/mgmt-api.txt#n3745 So I think it is better to maintain the current behavior and handle this in the daemon, meaning we shall handle if the Advertising got removed we just re-add it on power on, so at D-Bus level we allow application to register advertising instance regardless of the power state but while at the kernel level the instances are cleared on power off. We can actually introduce a property to tell if the instance is active or not at the kernel level so the daemon is able to notify the application when they are currently programmed in the controller or not. > > Thanks > Zhengping > > On Wed, Jan 25, 2023 at 11:02 PM Luiz Augusto von Dentz > <luiz.dentz@xxxxxxxxx> wrote: > > > > Hi Tedd, > > > > On Wed, Jan 25, 2023 at 8:34 PM Zhengping Jiang <jiangzp@xxxxxxxxxx> wrote: > > > > > > Hi Luiz, > > > > > > Thanks for the comment. Is there a way to get more information about > > > the failed test? After we cherry picked the hci_sync rework to our > > > branch, some existing tests failed. The failed test will register a > > > few advertisements, then run a power cycle. After the power cycle, the > > > advertisements should be re-enabled automatically. I believe this used > > > to be the behavior before the hci_sync rework? Now calling > > > "hci_clear_adv_sync" during hci_power_off_sync will remove all > > > advertisement instances, so the client code needs to "manually" > > > re-register the advertisement after powering on the adapter. Please > > > let me know if I understood correctly. > > > > What test are you talking about? The one in mgmt-tester which the CI > > runs or your own tests? > > > > @Tedd Ho-Jeong An Perhaps we could add the github PR link to pw that > > way people can inspect the errors. > > > > > Thanks, > > > Zhengping > > > > > > On Wed, Jan 25, 2023 at 3:47 PM Luiz Augusto von Dentz > > > <luiz.dentz@xxxxxxxxx> wrote: > > > > > > > > Hi, > > > > > > > > On Wed, Jan 25, 2023 at 2:06 PM <bluez.test.bot@xxxxxxxxx> wrote: > > > > > > > > > > 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=715641 > > > > > > > > > > ---Test result--- > > > > > > > > > > Test Summary: > > > > > CheckPatch PASS 0.65 seconds > > > > > GitLint FAIL 0.85 seconds > > > > > SubjectPrefix PASS 0.09 seconds > > > > > BuildKernel PASS 31.31 seconds > > > > > CheckAllWarning PASS 33.97 seconds > > > > > CheckSparse PASS 38.25 seconds > > > > > CheckSmatch PASS 107.12 seconds > > > > > BuildKernel32 PASS 29.96 seconds > > > > > TestRunnerSetup PASS 430.17 seconds > > > > > TestRunner_l2cap-tester PASS 16.16 seconds > > > > > TestRunner_iso-tester PASS 16.36 seconds > > > > > TestRunner_bnep-tester PASS 5.43 seconds > > > > > TestRunner_mgmt-tester FAIL 110.68 seconds > > > > > TestRunner_rfcomm-tester PASS 8.62 seconds > > > > > TestRunner_sco-tester PASS 7.99 seconds > > > > > TestRunner_ioctl-tester PASS 9.42 seconds > > > > > TestRunner_mesh-tester PASS 6.79 seconds > > > > > TestRunner_smp-tester PASS 7.81 seconds > > > > > TestRunner_userchan-tester PASS 5.73 seconds > > > > > IncrementalBuild PASS 27.75 seconds > > > > > > > > > > Details > > > > > ############################## > > > > > Test: GitLint - FAIL > > > > > Desc: Run gitlint > > > > > Output: > > > > > [kernel,v1,1/1] Bluetooth: Don't send HCI commands to remove adv if adapter is off > > > > > > > > > > WARNING: I3 - ignore-body-lines: gitlint will be switching from using Python regex 'match' (match beginning) to 'search' (match anywhere) semantics. Please review your ignore-body-lines.regex option accordingly. To remove this warning, set general.regex-style-search=True. More details: https://jorisroovers.github.io/gitlint/configuration/#regex-style-search > > > > > 1: T1 Title exceeds max length (82>80): "[kernel,v1,1/1] Bluetooth: Don't send HCI commands to remove adv if adapter is off" > > > > > ############################## > > > > > Test: TestRunner_mgmt-tester - FAIL > > > > > Desc: Run mgmt-tester with test-runner > > > > > Output: > > > > > Total: 494, Passed: 493 (99.8%), Failed: 1, Not Run: 0 > > > > > > > > > > Failed Test Cases > > > > > Add Advertising - Success 18 (Power -> off, Remove) Timed out 2.314 seconds > > > > > > > > Looks like there is something not quite right wrt assumption that > > > > power off don't remove clear the adv, at least this test says > > > > otherwise and this test is actually quite old so I don't know if it > > > > has always been like that or we did change this behavior and forgot to > > > > change the test, anyway this explains why we have done the clearing > > > > with cmd_sync work since that is what this test expects. > > > > > > > > > > > > > > --- > > > > > Regards, > > > > > Linux Bluetooth > > > > > > > > > > > > > > > > > -- > > > > Luiz Augusto von Dentz > > > > > > > > -- > > Luiz Augusto von Dentz -- Luiz Augusto von Dentz
