Skip to content

Conversation

@zxzxwu
Copy link
Contributor

@zxzxwu zxzxwu commented Aug 18, 2023

Feel free to modify

@zxzxwu
Copy link
Contributor Author

zxzxwu commented Aug 21, 2023

Same as #11, if there are multiple apps requiring SDP dicovery, should there be a singleton pattern for SDP?(Though this is probably a Bumble implementation detail)

@zxzxwu zxzxwu force-pushed the sdp branch 4 times, most recently from 557765f to 9b7fb91 Compare August 24, 2023 09:06
@zxzxwu zxzxwu marked this pull request as ready for review August 28, 2023 05:32
@uael uael requested review from DeltaEvo and uael September 15, 2023 22:39
rpc RemoveService(RemoveServiceRequest) returns (google.protobuf.Empty);
// List all registered SDP services
rpc ListAttributes(ListAttributesRequest) returns (ListAttributesResponse);
// Client methods
Copy link
Contributor

Choose a reason for hiding this comment

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

I suggest to remove the Connect and Disconnect APIs

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It would them be a problem: if there are multiple SDP connections on the same ACL connection, which one it should go?

// Register an SDP service
rpc AddService(AddServiceRequest) returns (google.protobuf.Empty);
// Remove an SDP service
rpc RemoveService(RemoveServiceRequest) returns (google.protobuf.Empty);
Copy link
Contributor

Choose a reason for hiding this comment

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

I would remove the RemoveService API and I'm not even sure we need to AddService as well 🤔

message ListAttributesRequest {}

message ListAttributesResponse {
repeated ServiceAttribute attributes = 1;
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
repeated ServiceAttribute attributes = 1;
map<uint32, repeated ServiceAttribute> attributes = 1;

}

message ServiceAttribute {
uint32 id = 1;
Copy link
Contributor

Choose a reason for hiding this comment

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

use an enum instead

// Remove an SDP service
rpc RemoveService(RemoveServiceRequest) returns (google.protobuf.Empty);
// List all registered SDP services
rpc ListAttributes(ListAttributesRequest) returns (ListAttributesResponse);
Copy link
Contributor

Choose a reason for hiding this comment

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

Any use case for this one ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

An usage is to avoid handle collision

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.

2 participants