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

Implement new return type for poll_at #216

Closed
wants to merge 2 commits into from
Closed

Implement new return type for poll_at #216

wants to merge 2 commits into from

Conversation

barskern
Copy link
Contributor

@barskern barskern commented May 16, 2018

Change return type for poll_at for sockets to be a PollAt instead of the former Option<Instant>.

pub(crate) enum PollAt {
    New,
    Time(Instant),
    Ingress,
}

NB! PollAt is for now just an internal type related to the sockets. I did not find a good place to put it if it should be a part of the public API. This means that the signature of the public function smoltcp::iface::ethernet::Interface::poll_at remains at

pub fn poll_at(&self, sockets: &SocketSet, timestamp: Instant) -> Option<Instant>

Maybe it should be considered to change this function to fit with the newly created type, however it would be a breaking change and would, IMO, require moving the implementation of PollAt to a different location.

See also: #215

Change return-type for `poll_at` for sockets to be a `PollAt` instead of
the former `Option<Instant>`.
@whitequark
Copy link
Contributor

@m-labs-homu r+

@m-labs-homu
Copy link

📌 Commit 786f043 has been approved by whitequark

m-labs-homu pushed a commit that referenced this pull request May 16, 2018
Change return-type for `poll_at` for sockets to be a `PollAt` instead of
the former `Option<Instant>`.

Closes: #216
Approved by: whitequark
@m-labs-homu
Copy link

⌛ Testing commit 786f043 with merge 805e11b...

@m-labs-homu
Copy link

💔 Test failed - status-travis

@barskern
Copy link
Contributor Author

barskern commented May 16, 2018

The failed tests on Travis are because of the function PollAt::is_ingress, which is currently only being used in the TcpSocket. I was thinking that this function will probably be useful in other contexts aswell to filter out non-interesting sockets. It is just to make that check a bit less verbose. I could add a attribute to conditionally compile it, but wouldn't that require unnecessary changes to the attributes if it will be used in the future?

Conditional compile vs #[allow(unused)]

@whitequark
Copy link
Contributor

Sorry, I misread. We typically use conditional compilation in these cases.

@whitequark
Copy link
Contributor

@m-labs-homu r+

@m-labs-homu
Copy link

📌 Commit e035aa5 has been approved by whitequark

@m-labs-homu
Copy link

⌛ Testing commit e035aa5 with merge 3fb0c22...

m-labs-homu pushed a commit that referenced this pull request May 16, 2018
Closes: #216
Approved by: whitequark
@m-labs-homu
Copy link

☀️ Test successful - status-travis
Approved by: whitequark
Pushing 3fb0c22 to master...

@barskern barskern deleted the feature/poll-at-type branch May 17, 2018 16:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants