Linkam T95 Unit Tests

[MikeHart85]

Closes #74.

This PR adds unit tests to test the Linkam T95 device at the class level.

[MichaelWedel]

As an unrelated idea, having seen this so often in this PR, could it be an option to add a convenience method for process(0)? Something like processEmpty() or processNone() or nullCycle() or...maybe you have a better idea :)

[MikeHart85]

Could just make 0 the default value for dt, no?

If we make a special function for it, the name should be as intuitive as possible to ensure its understood what it actually does. Especially since we're not making it any shorter, only longer. But I'm not sure any of those three options really describe what it does, nor I can think of a better option ... processZeroDeltaCycle()? Sounds horrible.

I kind of like it the way it is though. We're processing a cycle and our dt is 0: process([dt=]0).

There's nothing special about it, the delta just happens to be 0. I don't think I'd even make the delta optional / default to 0.

[MichaelWedel]

I'm not sure it isn't special, because in 23 of 36 cases in this file the argument is 0. Maybe thinking about the reason for doing this leads to a good name.

[MikeHart85]

Hm... Basically, I needed a cycle in order to process transitions, but I didn't want any time to pass.

But the only reason I didn't want any time to pass is to make it easier to synchronize when to assert the target temperature has been reached.

I could have just as well used 0.1 (or any other value shorter than the 60 second target runtime)... but then I would have had to do more math to calculate how long I need to process to reach (or not reach, or overshoot, etc) the target time/temperature, especially when multiple transitions needed to occur before I got to that point.

In some places, it doesn't even matter for timing. For example, the process calls immediately after creating the linkam instance and the ones following the getStatus immediately after instantiation. Those process calls I could have written process(0x1337BEEF) and it wouldn't have made any functional difference.

[MichaelWedel]

So the reason is to process transitions without passing time. The "any time is valid" is another pointer in my opinion. If time doesn't matter in a time based simulation then it probably has nothing to do with time and it shouldn't be necessary to supply a time to do what you want to do.

Do I understand correctly that it is basically "skipping to the next point where time matters again"?

[MikeHart85]

No, that's not correct.

The only thing that all process(0) calls have in common is that I want to process a cycle (specifically to trigger any transitions that may be valid, but that applies to all process calls regardless what the delta t parameter is) with a delta t of 0.

In some instances where process(0) is called, time does not matter at all, because it just so happens that the state the statemachine is in doesn't have any kind of time-based behaviour.

In some instances where process(0) is called, time does not matter for the purposes of the particular test I am running, because even though the state has time-based behaviour, it is neither behaviour I am testing nor does it have side-effects on behaviour I will subsequently test.

In some instances where process(0) is called, time matters and I want the delta to be 0 to make the delta in a subsequent process call both easier to calculate and clearer in intention for the reader. For example:

# Cool for almost a minute but not quite
linkam.process(59.5)

I could have used process(30) instead of process(0) in the previous three process calls. But then I would have had to write process(29.5) here.

The reason that three previous process(30) calls add up to a difference of -30 in the fourth is because the first two don't matter and the third one does.

The reason the first two don't matter is because I hadn't started cooling yet. They occurred in the different states, the behaviour of which I wasn't testing. The third one matters because I already started cooling and cooling is the state I'm testing here.

By using 0s in all three cases, I keep things simple and can focus on the forth -- the thing I'm actually testing.

[MichaelWedel]

The only thing that all process(0) calls have in common is that I want to process a cycle

Maybe something like nextCycle then. The importance of this is nowhere near the length of this discussion, I just thought it could be nice to make the intent of this more clear. After all process(0) could be implemented such that it is a no-op that doesn't result in going to the next cycle.

[MikeHart85]

After all process(0) could be implemented such that it is a no-op that doesn't result in going to the next cycle.

Hm... I would find this very surprising (to the point where I would raise an issue against it).

If I tell a system to process a cycle, I expect it to process a cycle (or throw an exception / return an error code). I suppose this comes back to something you said earlier:

If time doesn't matter in a time based simulation then [...]

It's not really a time based simulation. It's a cycle based simulation. Time is just a parameter that gets passed along "to whomever it may concern". When I call process(0), I'm saying: "Process a cycle. Oh, and by the way, if anyone asks, tell them dt is 0."

If that's the part that is confusing, maybe process itself isn't a good name?

But, as you say, it's questionable whether this is worth much effort. You would only call process manually in tests, anyway. It's called automatically during normal simulation. And while, normally, dt wouldn't be 0 since it loops and calculates a delta, there is nothing special about a delta of 0 either. The dt argument is -- and should only ever be -- used as a factor. Never as a direct condition (though things increased as a result of factoring in dts may of course be conditions).

[MikeHart85]

Thinking about this some more, I don't think this is that trivial. It bothers me that it bothered you, or that you even noticed it to begin with.

A process(0) call shouldn't have stood out at all. I would have thought anyone reading that would immediately see that it's doing a cycle with a delta of 0, and that there's nothing peculiar about that. A big part of the reason I don't really want a separate function for it is because there's nothing special about it and I don't want to send a message that there is.

If you thought it was weird or some sort of special case, others likely will as well. And we do expect (at least some) users will write unit tests for their devices, so they will (or should) be calling / seeing it too.

[MichaelWedel]

It's not really a time based simulation. It's a cycle based simulation.

I fell into the trap :) You're totally right, it's cycle-based not time based. I got myself quite confused apparently. After that reminder I actually agree that there shouldn't be a separate function for this, not at all, and yes, maybe we can find a better name instead of process.

You're also right about process never being called anywhere except in unit tests, but there it might be called with "I don't care" often (as seen here). So what about making dt optional, possibly with 0 as default?

I think even though this was off-topic for the PR it was good that we cleared that up somehow, if anything I re-learned that it's based on cycles and not time :D