| Message ID | 20210422192253.553048-1-marijns95@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [BlueZ] audio/avrcp: Determine Absolute Volume support from feature category 2 | expand |
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=471799
---Test result---
Test Summary:
CheckPatch FAIL 0.28 seconds
GitLint FAIL 0.12 seconds
Prep - Setup ELL PASS 48.51 seconds
Build - Prep PASS 0.11 seconds
Build - Configure PASS 8.34 seconds
Build - Make PASS 199.90 seconds
Make Check PASS 9.19 seconds
Make Dist PASS 12.99 seconds
Make Dist - Configure PASS 5.25 seconds
Make Dist - Make PASS 82.25 seconds
Build w/ext ELL - Configure PASS 8.39 seconds
Build w/ext ELL - Make PASS 189.51 seconds
Details
##############################
Test: CheckPatch - FAIL
Desc: Run checkpatch.pl script with rule in .checkpatch.conf
Output:
audio/avrcp: Determine Absolute Volume support from feature category 2
WARNING:COMMIT_LOG_LONG_LINE: Possible unwrapped commit description (prefer a maximum 75 chars per line)
#22:
[1]: https://android.googlesource.com/platform/system/bt/+/android-11.0.0_r28/bta/av/bta_av_main.cc#761
- total: 0 errors, 1 warnings, 18 lines checked
NOTE: For some of the reported defects, checkpatch may be able to
mechanically convert to the typical style using --fix or --fix-inplace.
"[PATCH] audio/avrcp: Determine Absolute Volume support from feature" has style problems, please review.
NOTE: Ignored message types: COMMIT_MESSAGE COMPLEX_MACRO CONST_STRUCT FILE_PATH_CHANGES MISSING_SIGN_OFF PREFER_PACKED SPLIT_STRING SSCANF_TO_KSTRTO
NOTE: If any of the errors are false positives, please report
them to the maintainer, see CHECKPATCH in MAINTAINERS.
##############################
Test: GitLint - FAIL
Desc: Run gitlint with rule in .gitlint
Output:
audio/avrcp: Determine Absolute Volume support from feature category 2
18: B1 Line exceeds max length (103>80): "[1]: https://android.googlesource.com/platform/system/bt/+/android-11.0.0_r28/bta/av/bta_av_main.cc#761"
##############################
Test: Prep - Setup ELL - PASS
Desc: Clone, build, and install ELL
##############################
Test: Build - Prep - PASS
Desc: Prepare environment for build
##############################
Test: Build - Configure - PASS
Desc: Configure the BlueZ source tree
##############################
Test: Build - Make - PASS
Desc: Build the BlueZ source tree
##############################
Test: Make Check - PASS
Desc: Run 'make check'
##############################
Test: Make Dist - PASS
Desc: Run 'make dist' and build the distribution tarball
##############################
Test: Make Dist - Configure - PASS
Desc: Configure the source from distribution tarball
##############################
Test: Make Dist - Make - PASS
Desc: Build the source from distribution tarball
##############################
Test: Build w/ext ELL - Configure - PASS
Desc: Configure BlueZ source with '--enable-external-ell' configuration
##############################
Test: Build w/ext ELL - Make - PASS
Desc: Build BlueZ source with '--enable-external-ell' configuration
---
Regards,
Linux Bluetooth
On Thu, 22 Apr 2021 at 21:23, Marijn Suijten <marijns95@gmail.com> wrote: > > The AVRCP spec (1.6.2) does not mention anything about a version > requirement for Absolute Volume, despite this feature only existing > since spec version 1.4. Android reports a version of 1.3 [1] for its > "AVRCP remote" (CT) service and mentions in the comment above it itself > relies on feature bits rather than the exposed version. As it stands > BlueZ requires at least version 1.4 making it unable to communicate > absolute volume levels with even the most recent Android phones running > Fluoride (have not checked the version on Gabeldorsche). > > The spec states that supporting SetAbsoluteVolume and > EVENT_VOLUME_CHANGED are mandatory when feature level 2 is declared, > excluded otherwise. This feature bit is set on Android and, when used > by this patch, allows for successfully communicating volume back and > forth despite the version theoretically being too low. > > [1]: https://android.googlesource.com/platform/system/bt/+/android-11.0.0_r28/bta/av/bta_av_main.cc#761 > --- > > Hi Luiz, Marek, > > It's been quite a while since our last mail contact. As mentioned > Android simply reports a too low version for its CT despite setting > category 2 for absolute volume support. Using this feature instead of > the version solves being unable to synchronize volume, is that okay with > you? > > - Marijn > > profiles/audio/avrcp.c | 7 +++++-- > 1 file changed, 5 insertions(+), 2 deletions(-) > > diff --git a/profiles/audio/avrcp.c b/profiles/audio/avrcp.c > index 05dd791de..bacd1aeb4 100644 > --- a/profiles/audio/avrcp.c > +++ b/profiles/audio/avrcp.c > @@ -4136,13 +4136,16 @@ static void target_init(struct avrcp *session) > (1 << AVRCP_EVENT_TRACK_REACHED_END) | > (1 << AVRCP_EVENT_SETTINGS_CHANGED); > > + if (target->features & AVRCP_FEATURE_CATEGORY_2) > + session->supported_events |= > + (1 << AVRCP_EVENT_VOLUME_CHANGED); > + > if (target->version < 0x0104) > return; > > session->supported_events |= > (1 << AVRCP_EVENT_ADDRESSED_PLAYER_CHANGED) | > - (1 << AVRCP_EVENT_AVAILABLE_PLAYERS_CHANGED) | > - (1 << AVRCP_EVENT_VOLUME_CHANGED); > + (1 << AVRCP_EVENT_AVAILABLE_PLAYERS_CHANGED); > > /* Only check capabilities if controller is not supported */ > if (session->controller == NULL) > -- > 2.31.1 > Hi Luiz, Has this patch by chance not reached your inbox? Would like to discuss the approach and get this issue fixed. Any suggestions on solving the test bot error? I don't think splitting too-long links over multiple lines is standard practice just to appease it? Thanks for looking into it. - Marijn
diff --git a/profiles/audio/avrcp.c b/profiles/audio/avrcp.c index 05dd791de..bacd1aeb4 100644 --- a/profiles/audio/avrcp.c +++ b/profiles/audio/avrcp.c @@ -4136,13 +4136,16 @@ static void target_init(struct avrcp *session) (1 << AVRCP_EVENT_TRACK_REACHED_END) | (1 << AVRCP_EVENT_SETTINGS_CHANGED); + if (target->features & AVRCP_FEATURE_CATEGORY_2) + session->supported_events |= + (1 << AVRCP_EVENT_VOLUME_CHANGED); + if (target->version < 0x0104) return; session->supported_events |= (1 << AVRCP_EVENT_ADDRESSED_PLAYER_CHANGED) | - (1 << AVRCP_EVENT_AVAILABLE_PLAYERS_CHANGED) | - (1 << AVRCP_EVENT_VOLUME_CHANGED); + (1 << AVRCP_EVENT_AVAILABLE_PLAYERS_CHANGED); /* Only check capabilities if controller is not supported */ if (session->controller == NULL)