tests: Bluetooth: Update Host Launch Studio Project and ICS #64239
Conversation
|
I updated audio profiles (and corresponding GAP requirements), and resolved MESH discrepancies (without enabling new features not v1.1) |
|
@sjanc sorry for not having reviewed this yet; working on something that I had hoped to finish by the weekend |
|
gentle ping BWT I have no idea why compliance is failing... |
Might be due to the size of the PR or something. One thing that GH shows is that |
|
@sjanc I'll try to see if I can review it by the end of this week. As we also previously discussed with @MariuszSkamra last Friday, it may make sense to go through this with an online meeting. Now sure how long that would take |
|
well, I'm not saying one person should review all profiles in project :-) review can be split I guess, ie host stuff (GAP, L2CAP, SM etc) I'm quite sure is OK |
|
compliance is not failing, but timing out after 6h of idling... I already restarted job couple times.. and bls is generated file so I didn't touch it, especially that launch studio is silently failing to import if it doesn't like bls (eg due to new lines changed) |
Have you tried running the Otherwise I'm fairly certain that we'll just manually override it if it cannot handle large files/large changes. |
|
I've run it locally and it has been hogging CPU on checkpatch for last 20 minutes with no progress Running DevicetreeBindings tests in /home/janc/devel/zephyr/zephyr ... |
Ya it's a bug in the checkpatch script. However, one of the files in this PR is indeed not passing compliance for it has DOS line endings. should get you going... if you also then add the missing commit sign-off :) |
|
missing sob added |
The .bls file is also still missing an final empty line it seems (or is that just Github messing about?) Regarding the file ending, the file ending in |
|
bls file is generated from and imported into Launch Studio portal, I don't think we should tamper it just to make CI happy |
|
gentle ping |
|
@sjanc I will go through this later today or tomorrow - Sorry for the delay |
There was a problem hiding this comment.
I don't think it makes sense to try and comment on the lines in the file, so you'll just the comments here:
I see that DIS is enabled, but not BAS which we also have in Zephyr. Is that intentional?
I see that OTS is enabled, but OTP is not. MCP uses OTP, so I guess OTP should be enabled at some point too.
MCS is also not enabled. Is that intentional?
Under "Unicast Server Requirements" the "12/17 Vendor-specific Codec Capability Setting" and "13/17 Vendor-specific Codec Capability Setting" are not enabled. I don't think we have anything stopping that from being enabled. There are a few other related things items that aren't enabled, which I think we could enable.
In Unicast Server Requirements Table 24 and Unicast Client Requirements Table 47, OOB supported is not enabled. I think we can support that in the host. (also for broadcast)
In Unicast Client Requirements Table 35, 36 and 37: We can also support vendor specific metadata
Table 58: LC3 Audio Configuration: Broadcast Source: We can enable 58/3
Table 60: GAP Requirements: Broadcast Source: I think we can enabled 60/4 as well
I see that PAST is disabled (e.g. 82/4), but we support that in the host, so we can run those tests with a non-Zephyr controller.
Table 3: Broadcast Audio Scan Service: we can enable autonomous syncing support as well
W.r.t. enabling the CAP Commander, it is still a work in progress so we won't be able to pass many tests yet, but should be nearly complete by end of year and done early 2024.
Similarly, the HAP HARC is also in PR now
For TMAP we support everything as well. If some things cannot be enabled due to lack of CAP Commander role, you can leave them out, but otherwise the remaining can be enabled.
For VOCS we support writable audio location and description as well as notifications
For GAP we also support Periodic Advertising with Responses and Periodic Advertising Connection, as well as Encrypted Data
For GAP (ISO) shouldn't 23/6, 23/7, 33/7 and 33/8 be enabled?
Should everything in GAP Table 30a: Central Link Layer Scanning Data Types be enabled? We support any data type.
For GATT, should 3a/1, 3a/2, 4a/1 and 4a/2 be enabled?
For OTS we support 3/2 and 3/3, 5/1, 5/2, 5/3, 5/4, 5/6 and 7/1
w.r.t. BAP, should we really enable support for both v1.0 and v1.0.1?
There was a problem hiding this comment.
Thanks for review! comments inline
I don't think it makes sense to try and comment on the lines in the file, so you'll just the comments here:
I see that DIS is enabled, but not BAS which we also have in Zephyr. Is that intentional?
Initially it was agreed that we do only GAP/GATT/SM/L2CAP/DIS from host profiles, there is also HRS,IAS and OTS. IAS and OTS are tested only because those are required by some LE Audio stuff.
BAS implementation seems very basic (only battery level while there are 46 (sic!) options in Table 2 in ICS configuration:P), I'll enable only mandatory stuff there. Same for HRS.
I see that OTS is enabled, but OTP is not. MCP uses OTP, so I guess OTP should be enabled at some point too.
Hmm at least consistency check isn't complaining about missing dependency and in MCP 'inter-layer dependency' there is only OTS listed, not OTP. I'll leave it disabled for now.
MCS is also not enabled. Is that intentional?
From what I see only GMCS is supported.
Under "Unicast Server Requirements" the "12/17 Vendor-specific Codec Capability Setting" and "13/17 Vendor-specific Codec Capability Setting" are not enabled. I don't think we have anything stopping that from being enabled. There are a few other related things items that aren't enabled, which I think we could enable.
OK, enabled
In Unicast Server Requirements Table 24 and Unicast Client Requirements Table 47, OOB supported is not enabled. I think we can support that in the host. (also for broadcast)
OK, enabled
In Unicast Client Requirements Table 35, 36 and 37: We can also support vendor specific metadata
OK, enabled
Table 58: LC3 Audio Configuration: Broadcast Source: We can enable 58/3
OK, enabled
Table 60: GAP Requirements: Broadcast Source: I think we can enabled 60/4 as well
OK, enabled
I see that PAST is disabled (e.g. 82/4), but we support that in the host, so we can run those tests with a non-Zephyr controller.
It was agreed that we enable only features that can be validated with open source controller.
Table 3: Broadcast Audio Scan Service: we can enable autonomous syncing support as well
OK, enabled
W.r.t. enabling the CAP Commander, it is still a work in progress so we won't be able to pass many tests yet, but should be nearly complete by end of year and done early 2024.
Similarly, the HAP HARC is also in PR now
It was agreed that we enable only things that are available in main branch. I think we can simple update after those are merged.
For TMAP we support everything as well. If some things cannot be enabled due to lack of CAP Commander role, you can leave them out, but otherwise the remaining can be enabled.
I believe I enabled everything that doesn't depend on either CAP Commander or BR/EDR
For VOCS we support writable audio location and description as well as notifications
OK, enabled
For GAP we also support Periodic Advertising with Responses and Periodic Advertising Connection, as well as Encrypted Data
Same as PAST, PAwR is not supported by open source controller.
Encrypted data enabled (where missing).
For GAP (ISO) shouldn't 23/6, 23/7, 33/7 and 33/8 be enabled?
I wasn't sure about those since they have LL dependency that is not enabled for 'host only' configuration and otherwise suppose to be excluded.
"C.1: Mandatory IF LL 9/31 "Connected Isochronous Stream Central", otherwise Excluded."
"C.1: Mandatory IF LL 9/32 "Connected Isochronous Stream Peripheral", otherwise Excluded."
Should everything in GAP Table 30a: Central Link Layer Scanning Data Types be enabled? We support any data type.
Yeap, I've miss that table, enabled now (excluded PAwR thing).
For GATT, should 3a/1, 3a/2, 4a/1 and 4a/2 be enabled?
Yeap, enabled.
For OTS we support 3/2 and 3/3, 5/1, 5/2, 5/3, 5/4, 5/6 and 7/1
Enabled. I also enabled 4/3 (dependency)
w.r.t. BAP, should we really enable support for both v1.0 and v1.0.1?
Yeah, I was wondering about this too but apparently v1.0 is mandatory, ie "BAP 1/1 Unicast Server" depends on it. Maybe it is just for compatibility testing with 1.0 device... ?
There was a problem hiding this comment.
I see that OTS is enabled, but OTP is not. MCP uses OTP, so I guess OTP should be enabled at some point too.
Hmm at least consistency check isn't complaining about missing dependency and in MCP 'inter-layer dependency' there is only OTS listed, not OTP. I'll leave it disabled for now.
OTP is optional for MCP, so maybe we missed something for object transport in MCP?
For GAP (ISO) shouldn't 23/6, 23/7, 33/7 and 33/8 be enabled?
I wasn't sure about those since they have LL dependency that is not enabled for 'host only' configuration and otherwise suppose to be excluded. "C.1: Mandatory IF LL 9/31 "Connected Isochronous Stream Central", otherwise Excluded." "C.1: Mandatory IF LL 9/32 "Connected Isochronous Stream Peripheral", otherwise Excluded."
Since they also don't seem to be required for BAP, I think we can leave them as is.
w.r.t. BAP, should we really enable support for both v1.0 and v1.0.1?
Yeah, I was wondering about this too but apparently v1.0 is mandatory, ie "BAP 1/1 Unicast Server" depends on it. Maybe it is just for compatibility testing with 1.0 device... ?
When I looked, it seemed like there was a message that it was being deprecated in 2025 or something like that - Sounds to mandate something that is deprecated. So if it is mandatory, that sounds like a bug to me, and IMO it makes more sense to only support 1.0.1 if we can, as I'm not sure our BAP implementation is fully compatible with v1.0.
There was a problem hiding this comment.
OTP is optional for MCP, so maybe we missed something for object transport in MCP?
MCP simply has no dependency on OTP in Launch Studio, only OTS.
Not sure what should be enabled in OTP though
w.r.t. BAP, should we really enable support for both v1.0 and v1.0.1?
Yeah, I was wondering about this too but apparently v1.0 is mandatory, ie "BAP 1/1 Unicast Server" depends on it. Maybe it is just for compatibility testing with 1.0 device... ?
When I looked, it seemed like there was a message that it was being deprecated in 2025 or something like that - Sounds to mandate something that is deprecated. So if it is mandatory, that sounds like a bug to me, and IMO it makes more sense to only support 1.0.1 if we can, as I'm not sure our BAP implementation is fully compatible with v1.0.
The comment is "To be deprecated", 1.0 is not deprecated yet.
There was a problem hiding this comment.
MCP simply has no dependency on OTP in Launch Studio, only OTS.
Not sure what should be enabled in OTP though
That's interesting - But I guess the MCP client has its own ICS values for objects.
The comment is "To be deprecated", 1.0 is not deprecated yet.
Fair point :)
I'd rather not go through all the things again to see if anything else is missing, so I'll approve as is. Any minor things can be fixed later.
There was a problem hiding this comment.
The comment is "To be deprecated", 1.0 is not deprecated yet.
Fair point :)
I'd rather not go through all the things again to see if anything else is missing, so I'll approve as is. Any minor things can be fixed later.
Yeap, I'd assume that this will be update every now and then (ie when major feature is merged or when new TCRL is released)
sjanc
force-pushed
the
tcrl-2023-1
branch
2 times, most recently
from
fd7a04f
to
fdd4786
Compare
This updates host Launch Studio Project and ICS to TCRL 2023-1. BLS file is deliberately left with Windows CRLF line terminators since otherwise Launch Studio silently fails to import such project. Signed-off-by: Szymon Janc <[email protected]>
|
removed characteristics from GAP service that are not supported by Zephyr |
|
@carlescufi The files in this PR are auto-generated but fails conformance because of that. I think we should overwrite the conformance check and merge this. |
This updates host Launch Studio Project and ICS to TCRL 2023-1.
BLS file is deliberately left with Windows CRLF line terminators since otherwise Launch Studio silently fails to import such project.