Ok, so I'll admit that I initially misunderstood the transitions of your Y/RED wave based on the sample
code you showed us.
And that the transitions are on each call to pulsein() vs measuring the time around each pulsein(),
my bad on that, however, I don't think it changes the interpretation of what is happening.
I guess where I'm at is that the graphs in the pictures you've shown so far do not seem indicate a bug with pulsein().
Those graphs appear to show pulsein() working just as it should - but.....
do seem to indicate that there are certain uses of pulsein() that worked in the past that may have
have issues with the new pulsein() now that the timeout mechanism was added to the pulse measurement in 0022.
My suspicion at this point it that, that are are 2 things that are contributing to your measurement
code not working as anticipated in the newer releases, and while it may make
your previously working implementation fail to work, unfortunately I wouldn't necessarily call it a bug in pulsein().
It is more of case where behavior has changed.
Take a look at the pictures you've provided.
While its a bit hard to see given the scale,
would you agree that the "non working" pulesin() example appears to show that the 2nd pulsein()
(as indicated but the first falling edge of Y) was just a bit later than
than the 2nd pulsein() in the "working" example based?
The width of the first red pulse is just a bit wider in the "non working" case.
In the "working" picture you can see that X is high is when Y/RED drops and that it is near
the front part of the high part of the X pulse.
In the "non working" picture, it appears that X is already low or just about to be low
when Y/RED drops.
If you don't agree, I'd like to understand the different interpretation.
Since there is some additional overhead between Y/RED dropping and pulsein() being called much
less being able to time the desired pulse, it appears in the "non working" example that the call to pulsein()
is not soon enough to to catch the first low pulse so it must wait for the 2nd low pulse.
So while the "non working" example isn't measuring the pulse you want, the picture
seems to show pulsein() measuring the first pulse it could given when pulsein() is being called.
So the real question becomes, why is the second call to pulsein() delayed in the non working case?
I can think of 2 off hand:
- using digitalWrite()
- pulsein() changed between 0021 and 0022 (comment shows granularity of width measurement doubled)
Lets look at each one:
digitalWrite()
Like I said before using digitalWrite() is not a good thing to use for any type of critical timing measurement
as it inserts overhead. LOTS of overhead vs doing direct port i/o.
Just because the digitalWrite() overhead is much shorter than your 40us period between pulses means nothing.
It could be that there is already 35 us of overhead before the pulsein() call is actually to the point in its internal code
of being able to start watching the signal line. In that case if digitalWrite() adds more than 5us of additional
overhead, your measuring code "breaks".
Keep in mind that when you see the output signal change, that there is still additional timing
overhead before the digitalWrite() returns.
digitalWrite() changed in 0019 to prevent port bit corruption when using digitalWrite() in ISRs
(I verfied that it was in 0019), That change makes digitalWrite() a little bit slower and also adds additional
overhead between the signal changing and the actual return of digitalWrite().
So after 0018 not only did digitalWrite() get a bit slower, but the latency between when a signal changes
and when the digitalWrite() returns has increased slightly.
My advice: NEVER, NEVER, NEVER use it for things like this.
pulsein() code changing
This is more than likely the biggest contributor to the failure.
The measurement code in pulsein() was changed in between 0021 and 0022
From this:
// wait for the pulse to stop
while ((*portInputRegister(port) & bit) == stateMask)
width++;
// convert the reading to microseconds. The loop has been determined
// to be 10 clock cycles long and have about 16 clocks between the edge
// and the start of the loop. There will be some error introduced by
// the interrupt handlers.
return clockCyclesToMicroseconds(width * 10 + 16);
to this:
// wait for the pulse to stop
while ((*portInputRegister(port) & bit) == stateMask) {
if (numloops++ == maxloops)
return 0;
width++;
}
// convert the reading to microseconds. The loop has been determined
// to be 20 clock cycles long and have about 16 clocks between the edge
// and the start of the loop. There will be some error introduced by
// the interrupt handlers.
return clockCyclesToMicroseconds(width * 21 + 16);
It was to add the timeout to the pulse measurement itself, to prevent the code
from hanging during pulse measurement.
While that seems innocent enough, it does affect how quickly pulsein() "notices" the pulse changing
which affects the granularity of the measurement and the latency before the pulsein() call returns.
One thing that concerns me is that the comment does not match the code.
comment says 20 and code uses 21 this extra +1 is not consistent with the previous loop
count of 10.
Regardless, the impact of the additional code is that it takes longer for the code to recognize a
transition. If it really is an additional 11 cycles over the previous code,
that is 11*62.5ns (using a 16mz clock) or 687.5ns is added to the granularity.
So while the old loop had a granularity of 625ns, the new loop has a granularity of 1312ns.
This means that you could potentially get stuck for up to an additional ~ 1.3us longer than
the real pulse length.
My guess as to what is happening here is that the new pulsein() granularity due to the timeout
capability combined with the dititalWrite() overhead, is creating just enough latency for the the 2nd pulsein()
call to just barely miss the start of desired pulse.
While technically, it isn't a "bug" it sure can cause previously working code to fail.
The documentation for pulseIn() really doesn't say how it handles the timeout of the
pulse itself. In fact the documentation on pulsein() currently clearly states that the timeout
is for waiting for the pulse to start and not for the pulse itself.
Having a granularity of 21 cycles seems like a lot and seems like it has the potential to break things,
like it seems to be doing in this case.
At a minimum, it creates additional error in the measurements.
Somebody thought that having a timeout for the actual pulse measurement was a good
thing and it got added in. However, it came with the cost of reducing the accuracy
of the measurements and everyone is now forced to live with this even if they
intentionally don't specify a timeout.
I guess the pulsein() code could be smarter when not using a timeout
so that it branched around to a different set of code that didn't do the timeout checks
vs just creating a really long timeout.
This would give better granularity and lower latencies with added risk of getting stuck
waiting for the signal to change.
pulseIn() could also be extended to have two timeouts,
one for the amount of time waiting for the pulse and one for the pulse itself to allow
the user to have better control over the timeouts.
i.e.
pulseIn(pin, value);
pulseIn(pin, value, waitforpulsetimeout);
pulseIn(pin, value, waitforpulsetimeout, pulselengthtimeout);
and then be smart enough to use code that does not do the checks when timeouts are not specified
so that better granularity and accuracy could be achieved.
I think going with double timeouts would offer the best flexibility and preserve the old
(pre 0022) behavior while adding the option for a timeout on the actual pulse measurement
all at the option of the user.
But the code has to be re-written to be smarter or potentially simply have 3 different routines
so that when timeouts are not specified, the pin monitoring loops are tighter.
--- bill