Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add: set multiple MAC in one BTDEVADDR feature #424

Merged

Conversation

hanhsuan
Copy link
Contributor

@hanhsuan hanhsuan commented Apr 7, 2023

Description

Describe your changes here:

BTDEVADDR could not set multiple BT MAC addresses at the same time. It will make automatic testing failed, if the test BT server be occupied or not available.

The only impact job is com.canonical.plainbox::bluetooth/bluetooth_obex_send.

Setting of BTDEVADDR is changing from

BTDEVADDR = 28:3A:4D:46:79:C0

to

BTDEVADDR = 28:3A:4D:46:79:C0,7C:B2:7D:4B:14:95,34:6F:24:A8:93:EE,80:32:53:D8:0D:1E

Resolved issues

Documentation

  • Automated tests are included for the changed functionality in this PR. If to be merged without tests, please elaborate why.
  • Necessary documentation is provided for the changed functionality in this PR (tests are documentation, too).

Tests

@hanhsuan hanhsuan requested a review from pieqq April 7, 2023 08:48
Copy link
Contributor

@kissiel kissiel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good. One minor English fix below.

providers/base/units/bluetooth/jobs.pxu Outdated Show resolved Hide resolved
@hanhsuan
Copy link
Contributor Author

hanhsuan commented Apr 7, 2023

I'm so sorry I rebased to wrong commit. Should I drop this PR then create a new one?

@kissiel
Copy link
Contributor

kissiel commented Apr 8, 2023

git reset the branch to the commit you want to keep, and repush your branch. This will delete the unwanted ones.

If there's problem with your order then you can reset your branch to the same commit as the main ref is pointing to and then cherry-pick only the commits you want to have. And then of course repush.

@hanhsuan hanhsuan force-pushed the Add_multiple_MAC_in_one_BTDEVADDR_feature branch from 65a5c1a to b7988e2 Compare April 10, 2023 00:44
@hanhsuan hanhsuan force-pushed the Add_multiple_MAC_in_one_BTDEVADDR_feature branch from b7988e2 to bed8915 Compare April 10, 2023 04:58
@hanhsuan
Copy link
Contributor Author

I have fixed it. Thanks for you help.

yphus
yphus previously requested changes Apr 10, 2023
Copy link
Contributor

@yphus yphus left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

small typo

providers/base/units/bluetooth/jobs.pxu Outdated Show resolved Hide resolved
Copy link
Collaborator

@pieqq pieqq left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for providing a link to all the different test cases you've tested, it's really helpful!

Looks good to me.

@pieqq pieqq dismissed yphus’s stale review April 21, 2023 06:23

change requests have been addressed

@pieqq pieqq merged commit 931e3ea into canonical:main Apr 21, 2023
@hanhsuan hanhsuan deleted the Add_multiple_MAC_in_one_BTDEVADDR_feature branch June 8, 2023 03:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants