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

X2 Dome interface protocol buffer WIP #10

Closed
wants to merge 2 commits into from

Conversation

AnthonyHorton
Copy link
Member

I've started an attempt to translate the TSX X2 Dome interface specification into protocol buffer .proto format so that we can use the grpc tools to generate template code for the X2 plugin & Raspberry Pi code.

@AnthonyHorton AnthonyHorton changed the title X2 Done interface protocol buffer WIP X2 Dome interface protocol buffer WIP May 10, 2019
@lspitler lspitler mentioned this pull request May 10, 2019
3 tasks
Copy link
Contributor

@wtgee wtgee left a comment

Choose a reason for hiding this comment

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

Seems straightforward.


service X2Dome {
rpc GetAzEl (Empty) returns (AzEl);
rpc GotoAzEl (AzEl) returns (Empty);
Copy link
Contributor

Choose a reason for hiding this comment

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

The spec doesn't call for it but do we want to be returning a common status object or something for all these?

Copy link
Member Author

Choose a reason for hiding this comment

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

Probably. Have been pondering how best to handle the fact that this interface definition is coming from old school C-ish C++ code where every function returns an integer, which may or may not be meaningful, and all the substantial return values are retrieved by passing in pointers as arguments. Probably should include the integer return value in the response 'message' for all of these.

rpc Open (Empty) returns (Empty);
rpc Close (Empty) returns (Empty);
rpc Park (Empty) returns (Empty);
rpc UnPark (Empty) returns (Empty);
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
rpc UnPark (Empty) returns (Empty);
rpc Unpark (Empty) returns (Empty);

The spec doesn't capitalize the 'P' here. Feel free to ignore if you wanted it this way though.

Copy link
Member Author

Choose a reason for hiding this comment

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

Oops. No, was trying to copy the formatting from the interface definition.

rpc OpenComplete (Empty) returns (IsComplete);
rpc CloseComplete (Empty) returns (IsComplete);
rpc ParkComplete (Empty) returns (IsComplete);
rpc UnParkComplete (Empty) returns (IsComplete);
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
rpc UnParkComplete (Empty) returns (IsComplete);
rpc UnparkComplete (Empty) returns (IsComplete);


import "google/protobuf/empty.proto";

service X2Dome {
Copy link
Contributor

Choose a reason for hiding this comment

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

Ideally this could be generic and not just X2, right? Wondering if we should make the class (and file) name more generic.

Copy link
Member Author

Choose a reason for hiding this comment

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

Well, in a way it is generic in as much as these are the sorts of RPC's, requests and responses that you might expect most domes to use. This protocol buffer file is intended to correspond directly to the X2 plugin Dome interface though, so that the template client code generated by the C++ version of the grpc tools could be used to write an actual X2 plugin.

@lspitler
Copy link
Member

@AnthonyHorton how can @fergusL and I move this forward? Is it just running your initial config using grpc?

@fergusL fergusL marked this pull request as ready for review July 3, 2019 08:00
@fergusL
Copy link
Contributor

fergusL commented Jul 3, 2019

I'm just going to merge this and open a new PR with the stuff I've done on top of this

@fergusL fergusL closed this Jul 3, 2019
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.

None yet

4 participants