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

Added GATT status code to a BleDisconnectException #405

Merged
merged 4 commits into from
Jun 8, 2018

Conversation

uKL
Copy link
Collaborator

@uKL uKL commented Mar 26, 2018

  • Added GATT status to description mapping

Fixes #282

@uKL uKL self-assigned this Mar 26, 2018
@uKL uKL requested a review from dariuszseweryn March 26, 2018 09:02
/**
* Set when the state is not available, for example when the adapter has been switched off.
*/
public static final int UNKNOWN_STATE = -1;
Copy link
Owner

Choose a reason for hiding this comment

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

Shouldn't it be status? And maybe STATUS_UNAVAILABLE?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

You're right.

}

public BleDisconnectedException(Throwable throwable, @NonNull String bluetoothDeviceAddress) {
super(createMessage(bluetoothDeviceAddress), throwable);
public BleDisconnectedException(Throwable throwable, @NonNull String bluetoothDeviceAddress, int state) {
Copy link
Owner

Choose a reason for hiding this comment

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

status
Plus this changes the public API :/

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes, should be status. Do you think that we should keep exception constructors as a part of the public API? If so, I can create a new one and deprecate the old one.

Copy link
Owner

Choose a reason for hiding this comment

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

We could deprecate the current constructors and add a LIBRARY_GROUP annotation to the new one

}

public BleDisconnectedException(@NonNull String bluetoothDeviceAddress) {
super(createMessage(bluetoothDeviceAddress));
public BleDisconnectedException(@NonNull String bluetoothDeviceAddress, int state) {
Copy link
Owner

Choose a reason for hiding this comment

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

status, public api

}

private static String createMessage(@Nullable String bluetoothDeviceAddress) {
return "Disconnected from " + bluetoothDeviceAddress;
private static String createMessage(@Nullable String bluetoothDeviceAddress, int state) {
Copy link
Owner

Choose a reason for hiding this comment

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

status, public api

@jreehuis
Copy link

Any news here? In our project it really helped us to tackle some requirements.

@uKL uKL force-pushed the feature/282_added_status_to_exception branch from 6d3dfb9 to 3ae9ce3 Compare June 4, 2018 09:52
@uKL
Copy link
Collaborator Author

uKL commented Jun 4, 2018

@dariuszseweryn I applied the required changes. Could you have a look?

Copy link
Owner

@dariuszseweryn dariuszseweryn left a comment

Choose a reason for hiding this comment

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

Afaik we're not into breaking the API yet


@Deprecated
public BleDisconnectedException() {
Copy link
Owner

Choose a reason for hiding this comment

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

I know it is probably deprecated long enough but it is still a breaking API change

@uKL uKL force-pushed the feature/282_added_status_to_exception branch from a2e1368 to 6b4dbea Compare June 4, 2018 12:20
@uKL uKL force-pushed the feature/282_added_status_to_exception branch from 6b4dbea to 592dd19 Compare June 8, 2018 06:24

public static BleDisconnectedException adapterDisabled(String macAddress) {
Copy link
Owner

Choose a reason for hiding this comment

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

This could be on the top level — not a nested class. Better sustainability I suppose.

@dariuszseweryn dariuszseweryn added this to the 1.7.0 milestone Jun 8, 2018
@uKL uKL merged commit 5787e3c into master Jun 8, 2018
@uKL uKL deleted the feature/282_added_status_to_exception branch June 8, 2018 08:54
dariuszseweryn pushed a commit that referenced this pull request Jul 6, 2018
* Added GATT status code to BleDisconnectException (#405)

* Added suggested RestrictTo LIBRARY_GROUP annotation fo adapterDisabled utility function.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants