Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

feat: implement Speaker and Serdes#6

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged
deansheather merged 8 commits intomainfromdean/speaker
Nov 27, 2024
Merged

Conversation

deansheather
Copy link
Member

@deansheatherdeansheather commentedNov 21, 2024
edited
Loading

Closes#3

  • Adds protobuf code generation
  • Adds utilities and wrapping code around RPC message types
  • Adds RPC protocol code required for sending and receiving messages
  • Adds a end-to-end test with messages and replies between two Speakers

TODO:

  • Add thecsharp_namespace option to the upstreamvpn.proto
  • Decide between event delegates (callbacks) or channels for receiving messages
  • PlumbCancellationTokenSource into more operations (any asyncSpeaker operation should use the suppliedCancellationToken as well as the globalCancellationTokenSource)
  • Make the startup process async?
  • Try to declutter the generic type specifications around the codebase
    • It may be possible to condense the twoTS/TSi andTR/TRi into justTS andTR with some finangling and changes around theRpcMessage<T> type
      • Maybe switch (back) to an interface rather than an abstract class
  • Move the role to be attached to the type so it doesn't need to be passed in manually to theSpeaker at all
  • Tests
    • ApiVersion
    • RpcRole
    • RpcHeader
    • RpcMessage (and RpcRoleAttribute)
    • Serdes
    • Speaker

- Adds protobuf code generation- Adds utilities and wrapping code around RPC message types- Adds RPC protocol code required for sending and receiving messages- Adds a end-to-end test with messages and replies between two Speakers
@deansheatherdeansheather self-assigned thisNov 21, 2024
The RpcMessage<T> type is now implemented directly on the protobuf types using partial classes.
@deansheatherdeansheather marked this pull request as ready for reviewNovember 26, 2024 04:34
Copy link
Member

@ethanndicksonethanndickson left a comment

Choose a reason for hiding this comment

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

I'm happy with the repo structure, the namespaces, and I of course agree that the C# code should be idiomatic C# and not just a translation of the Go code. FWICT the C# API looks good.

LGTM, aside from the earlierRPC.MsgID thread - can probably just merge this for now?

/// A task that completes when all tasks are completed, with the cancellation or exception state of the first
/// non-successful task
/// </returns>
public static async Task CancellableWhenAll(CancellationTokenSource cts, params Task[] tasks)

Choose a reason for hiding this comment

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

nice!

Copy link
Collaborator

@spikecurtisspikecurtis left a comment

Choose a reason for hiding this comment

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

Some inline comments, but overall, I like the shape of the APIs to the protocol.

@deansheatherdeansheather merged commit08487ac intomainNov 27, 2024
@deansheatherdeansheather deleted the dean/speaker branchDecember 9, 2024 09:06
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers

@spikecurtisspikecurtisspikecurtis approved these changes

@ethanndicksonethanndicksonethanndickson approved these changes

Assignees

@deansheatherdeansheather

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Implement the CoderVPN protocol
3 participants
@deansheather@spikecurtis@ethanndickson

[8]ページ先頭

©2009-2025 Movatter.jp