Giter Club home page Giter Club logo

Comments (20)

thomasfredericks avatar thomasfredericks commented on July 17, 2024 1

There is already a restart(). I believe this might bring feature fatigue.

from chrono.

sofian avatar sofian commented on July 17, 2024

It would be useful to have something of the sort. Perhaps the right way to do it would be to have stop() do what you suggest, and replace the current stop() function by pause(). This would break existing code, though. Another option would be to give it as an option to stop() eg. void stop(bool reset=false); so if you want to reset you can call stop(true); What do you think @thomasfredericks ?

from chrono.

thomasfredericks avatar thomasfredericks commented on July 17, 2024

I suggest to not break the public interface.

Also, I do not like something like stop(true); because it is unclear what we are setting to true.

But since calling start() sets the offset to 0, doesn't it already reset the counter to 0 when started?

For the pause functionality we already have resume().

What is the user case for a Chrono::stop_reset() ?

For reference:

void Chrono::start(Chrono::chrono_t offset) {
  restart(offset);
}

void Chrono::restart(Chrono::chrono_t offset) {
  _startTime = _getTime();
  _offset    = offset;
  _isRunning = true;
}

void Chrono::resume() {
  if (!_isRunning) { // can only resume if was stopped
    _startTime = _getTime();
    _isRunning = true;
  }
}

from chrono.

sofian avatar sofian commented on July 17, 2024

I think the problem is that after calling stop() the functions elapsed() and hasPassed() do not return to zero. In some circumstances that can be confusions.

My suggestion: add a function called reset(). It makes sense that a reset() function would also stop the chronometer.

from chrono.

sofian avatar sofian commented on July 17, 2024

@riodda Can you give an example of why you think that feature is important to have?

from chrono.

sofian avatar sofian commented on July 17, 2024

@thomasfredericks restart() does not do the same thing, as it starts the timing process. Indeed, there is currently no way to reproduce the initial state of a non-started Chrono object beyond re-calling the constructor.

from chrono.

sofian avatar sofian commented on July 17, 2024

Normally real-life chronometers have a reset() function that brings back the counter to 00:00:00. So it would not be too counter-intuitive to have it. I think it makes sense.

from chrono.

22chrs avatar 22chrs commented on July 17, 2024

This would be really helpful. I have the same problem and I want to reset the Chrono at a given time. My example is, that I want to give the user feedback on a display, that the interval time is starting again. In the first place it was really confusing until I got it, that the restart is not reseting the chrono to 0 again. So having a reset would be really intuitive.
Anyway. Thanks for your great work! :)

from chrono.

sofian avatar sofian commented on July 17, 2024

The restart() is resetting the chrono to 0 (basically start() and restart() do the same thing).
We should have had a pause() function from the beginning, and stop() should have reset timer to zero. But at this point I agree we should not break the public interface.

So perhaps a reset() function would be the way to go. I would support adding something like this.

void reset(chrono_t offset=0) {
  _offset = offset; // reset to offset
  _isRunning = false;
}

from chrono.

thomasfredericks avatar thomasfredericks commented on July 17, 2024

This would be really helpful. I have the same problem and I want to reset the Chrono at a given time. My example is, that I want to give the user feedback on a display, that the interval time is starting again. In the first place it was really confusing until I got it, that the restart is not reseting the chrono to 0 again. So having a reset would be really intuitive. Anyway. Thanks for your great work! :)

I do not understand. How is restart not setting the Chrono to 0? It should! What do you need?

from chrono.

sofian avatar sofian commented on July 17, 2024

@thomasfredericks It's true: restarts() does resets the Chrono to zero -- but it also starts the Chrono. So it is not possible to stop the Chrono (_isRunning = false) and set it to zero (or even, set it to some offset value).

Think about an actual chronometer to time your 100m sprint or whatever. You cannot set it back to zero without actually starting it. I think there are applications where this is something you would want to do. In other words: to me, we are missing the possibility to set the chrono to zero (or some other offset) while also stopping it. We do not have that option right now.

from chrono.

thomasfredericks avatar thomasfredericks commented on July 17, 2024

Stop, pause, and play/start behave differently depending on the implementation, the type of device and the manufacturer.

Start
In some cases start continues from a pause. Sometimes start restarts the count from 0.
Pause
Sometimes pause acts as a toggle between a run and not run state. Sometimes it pauses but can't un-pause.
Stop
In some cases stop disables the run state and sets the counter to zero. In other cases it acts as another pause.

In most documentation and examples of the Chrono library, restart is used instead of start as it explicitly states that the counter will be reset to 0 AND the run state will be activated .

Based on wanting to keep this, the other methods could be distinguished as so:

  • stop could deactivate the run state and set the counter to 0
  • pause could deactivate the run state but no set the counter to 0
  • start could activate the run but not reset the counter to 0

Since start could be confused with restart it could be deprecated and renamed continue (but we already have resume).

from chrono.

thomasfredericks avatar thomasfredericks commented on July 17, 2024

See the documentation here that uses restart() : http://sofapirate.github.io/Chrono/

from chrono.

sofian avatar sofian commented on July 17, 2024

Based on wanting to keep this, the other methods could be distinguished as so:

  • stop could deactivate the run state and set the counter to 0
  • pause could deactivate the run state but no set the counter to 0
  • start could activate the run but not reset the counter to 0

Since start could be confused with restart it could be deprecated and renamed continue (but we already have resume).

I think this would be (almost) ideal. But it would break both the behavior of stop() and start() -- so it would not be back-compatible. I see two alternatives:

First proposal (not back-compatible because change to stop() function):

  • stop deactivate the run state and set the counter to 0 (changed)
  • pause deactivate the run state but no set the counter to 0 (new)
  • resume reactivate the run state and no set the counter to 0 (unchanged)
  • start activate the run and reset the counter to 0 (unchanged)

Second proposal (back-compatible):

  • reset set the counter to 0 -- possibly also deactivate the run state (new)
  • stop deactivate the run state but not set the counter to 0 (unchanged) (equivalent of pause() from 1st proposal)
  • resume reactivates the run state and no set the counter to 0 (unchanged)
  • start activates the run and resets the counter to 0 (unchanged)

from chrono.

thomasfredericks avatar thomasfredericks commented on July 17, 2024

The word reset is too ambiguous. Also as the default behaviour of Chrono is to start running on creation, a reset would actually mean to restart running.

from chrono.

thomasfredericks avatar thomasfredericks commented on July 17, 2024

A better term could be ready as when runners align on the starting line as in the phrase ready, set, go!.

from chrono.

sofian avatar sofian commented on July 17, 2024

Interesting but ready is also a bit vague -- it could also give the impression this is a boolean function.
Another option: we could simply add a function set(chronot_t offset) which would not change the running state but just set current time to a certain value. But then to do a "reset" you need to call set(0). We could make the offset to zero by default so by calling set() would set the clock to 0 -- but it's a big vague.

IMHO the "stop, pause, resume, start" is the clearest approach. It would break back-compatibility so we'd have to be ready for it and it could mean a period of adjustment, but I think it would be my preference, better to do it now than later.

We could (and should) add a set(chronot_t offset) function as well cause it can be useful (right now we only have an add(chronot_t offset) option). But that's another discussion.

from chrono.

thomasfredericks avatar thomasfredericks commented on July 17, 2024

I would just modifiy the behavoir of stop() (so it resets to 0) and add a new pause() (and probably have to add a togglePause() because someone will expect it to toggle) but then things begin to get complicated so set() is a good compromise. As long as we don't add confusion to or change the behavior of the basic functions elapsed(), restart() and hasPassed().

For @22chrs , the simplest solution would be to just check if the Chrono is running:

if ( myChrono.isRunning() ) Serial.println(0);
else Serial.println(myChrono.elapsed());

from chrono.

sofian avatar sofian commented on July 17, 2024

@thomasfredericks Ok, so if I understand right, this would correspond to my first proposal, so it seems we agree:

  • change behavior of stop() so it resets counter to zero
  • add a pause() function

I don't think we need a tooglePause() function because resume() is perfectly consistent IMHO.

But we should definitely add a set() function which seems to be the missing part in our update.

If we do these updates I would suggest we move to version 2.0 because we are breaking compatibility with current version.

from chrono.

thomasfredericks avatar thomasfredericks commented on July 17, 2024

@sofian, yes I agree.

from chrono.

Related Issues (11)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.