search

pine64 / OpenPineBuds

Community maintained firmware for PineBuds Pro
150 stars 19 forks source link

anc: Fix enable/disable from slave bud #74

Closed Haxk20 closed 1 year ago

Haxk20 commented 1 year ago

Switching ANC on or off from master bud has always worked since we could just straight up enable it and no command had to be sent.

When switching from slave bud we had to send the command to enable or disable it via app_anc_key. Issue is the handling of this request is from app_anc_cmd_receive_process which is not called without APP_ANC being defined.

This commit cleans up the way ANC is enabled and also fixes slave bud ANC switching by implementing the actual code path :)

Please note that the ANC testing branch should be rebased on top of main with this commit added.

I did my testing with main merged with anc_testing branch thus why the key_handling.cpp file :)

shymega commented 1 year ago

This PR should be targeted at anc_testing branch, not main. Voting to close in favour of a new PR based off anc_testing, which can then be merged cleanly when anc_testing branch is ready.

FintasticMan commented 1 year ago

The base branch of a PR can be changed without needing to close and re-open.

shymega commented 1 year ago

The base branch of a PR can be changed without needing to close and re-open.

@FintasticMan Yes, this is true. I can't do it myself though.

Ralim commented 1 year ago

I changed the PR target branch, though looks like this wasnt a clean base off that branch. Could you please rebase your patch onto the anc-testing branch?

shymega commented 1 year ago

I can do it from my end.

shymega commented 1 year ago

Done, need approval to push.

shymega commented 1 year ago

Done, discussed with @Haxk20 on channel, and I've updated the PR.

Ralim commented 1 year ago

This looks great, very happy for this fix to go in. Though I should probably update the anc_testing branch as well. Ill try and get that done tonight which will probably make this neater to merge; apologies.