Skip to content

Conversation

@cheatfate
Copy link
Collaborator

@cheatfate cheatfate commented Mar 24, 2019

DatagramCallback* = proc(transp: DatagramTransport,
                         remote: TransportAddress): Future[void] {.gcsafe.}
                         message: seq[byte],
                         remote: TransportAddress,
                         error: OSErrorCode): Future[void]

message - UDP message bytes.
If len(message) == 0 you need to check OSErrorCode to obtain error code.


DatagramCallback* = proc(transp: DatagramTransport,
remote: TransportAddress): Future[void] {.gcsafe.}
message: seq[byte],
Copy link
Member

Choose a reason for hiding this comment

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

openArray[byte] tends to give more flexibility for the implementation.. a common option later on is to allow the original caller to set up a byte buffer of their choice..

Copy link
Member

Choose a reason for hiding this comment

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

this is also the kind of place where Result is appropriate - giving either an error or a buffer but never both - makes for cleaner code everywhere

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@arnetheduck we can't use openarray[T] because it can't be captured inside of iterator, so it can't work in async procedures.

remote: TransportAddress): Future[void] {.gcsafe.}
message: seq[byte],
remote: TransportAddress,
error: OSErrorCode): Future[void] {.gcsafe.}
Copy link
Contributor

Choose a reason for hiding this comment

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

Seems like error which can happen here is either not realistic or unrelated to remote-specific message receiving. Thus I would consider removing the error from this function signature. For remote-unrelated events I would introduce another callback like onNetworkChanged.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Sorry, but chronos is cross-platform framework, and if Linux has small number of errors, BSD/MacOS and Windows has much more, so i wish to leave error code here.

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.

3 participants