Skip to content

[Breaking] New polledtimeout #7960

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

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open

Conversation

devyte
Copy link
Collaborator

@devyte devyte commented Apr 4, 2021

Fixes #7903

This PR restructures the polledtimeout code by separating the templates from the policies, which makes it easier for users to understand how to extend the code with new custom policies. It also separates* the generic policies that should be common to all Arduino cores from the policies that are specific to the esp8266.

The namespaces and template name have been changed to uppercase based on multiple feedback received since the frist version was implemented.

As usual, the specifics are up for discussion.

  • Restructure polledtimeout
  • simplify namespaces
  • rename across code
    • new methods remaining(), elapsed(), getters

@devyte devyte changed the title new polledtimeout [Breaking] New polledtimeout Apr 4, 2021
@d-a-v d-a-v added this to the 3.0.0 milestone Apr 5, 2021
@d-a-v
Copy link
Collaborator

d-a-v commented Apr 5, 2021

Thanks !
Is it a strong breaking change or do you plan to add something like this, for users who would have made the effort to use it ?

namespace esp8266
{
    using  polledTimeout = ::PolledTimeout [[ deprecated, "use PolledTimeout instead of esp8266::polledTimeout" ]];
}

This fixes #7903

@d-a-v d-a-v linked an issue Apr 5, 2021 that may be closed by this pull request
@devyte
Copy link
Collaborator Author

devyte commented Apr 6, 2021

Is it a strong breaking change or do you plan to add something like this, for users who would have made the effort to use it ?

namespace esp8266
{
    using  polledTimeout = ::PolledTimeout [[ deprecated, "use PolledTimeout instead of esp8266::polledTimeout" ]];
}

I intend this to be a strong breaking change. I don't want the legacy naming to be dragged into the future.

@devyte devyte marked this pull request as ready for review April 6, 2021 00:39
@devyte devyte self-assigned this Apr 6, 2021
@devyte devyte requested a review from d-a-v April 6, 2021 02:21
@dok-net
Copy link
Contributor

dok-net commented Apr 7, 2021

@devyte The copyright header is AWOL in PolledTimeout.h, the three newly added files seem OK in this regard.


// legacy type names, deprecated (unit is milliseconds)
using oneShot = TimeoutTemplate<false, YieldPolicy::DoNothing, TimePolicy::TimeMillis>;
using periodic = TimeoutTemplate<true, YieldPolicy::DoNothing, TimePolicy::TimeMillis>;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why are these kept in this breaking change commit ?

@d-a-v d-a-v modified the milestones: 3.0.0, 3.0.1 May 16, 2021
@d-a-v d-a-v modified the milestones: 3.0.1, 3.1 Jun 16, 2021
@d-a-v d-a-v modified the milestones: 3.1, 4.0.0 Jun 13, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Feature Request: simplify PolledTimeout namespace
3 participants