allwpilib
allwpilib copied to clipboard
wpilibsuite
DigitalOutput pulse doesn't scale linearly with input
Describe the bug
When calling DigitalOutput.pulse(); the RIO outputs pulses of arbitrary lengths that don't correlate with the input value.
To Reproduce Steps to reproduce the behavior:
- Create a DigitalOutput
DigitalOutput digitalOutput = new DigitalOutput(0); - (Not sure if this is necessary but our code has it) Set the digitalOutput to false
digitalOutput.set(false); - Pulse the digitalOutput
digitalOutput.pulse(20);
Expected behavior We expected the output to to scale with input value (eg. increasing input value would increase pulse length) but instead the pulse is of an arbitrary length that doesn't reflect changes of input value.
Screenshots Pulse output length was measured in milliseconds using an oscilloscope with different input values. We didn't take photos of the oscilloscope screen but our data is here below: https://docs.google.com/spreadsheets/d/1m2L3OeYdsEEMhN6ocPFskpC3ENEAc3giSgxebHkpqXE/edit?usp=sharing
Desktop (please complete the following information):
- WPILib Version: [e.g. 2021.3.1]
Version: 2022.4.1 - OS: [e.g. Windows 11]
Windows 10 - Java version [e.g. 1.10.2]
11.0.15 - C++ version [e.g. 17]
N/a
Additional context None
The pulse output duration is in seconds. Did you really intend to generate a 20 second pulse (versus 20 milliseconds, in which case you should pass 0.020)? Internally the pulse duration is truncated to a 16-bit integer after scaling by the PWM loop timing, so what you are seeing is a result of this truncation. https://github.com/wpilibsuite/allwpilib/blob/faa29d596c9a01447a36fd5eaa18e8ad28434ad4/hal/src/main/native/athena/DIO.cpp#L428-L429
If you really need a 20 second pulse, you should be able to trade total duration against pulse accuracy by decreasing the PWM loop timing to keep the scaled pulse duration value within 16 bits.
Though originally, I had wanted it to be a 20ms pulse I found out that the pulse length wouldn't correlate to a linear change in the pulse output. So even if I had put in 20 it still should have shown 20 of something instead it would pulse for a completely random duration in ms.
If you put anything over then I think 0.5 the result will be random. It depends on where in the rollover the value you use actually is. I’d have to check the exact maximum, but iirc it’s somewhere around half a second.
Even with a number below 0.5 I still couldn't get it to pulse for a given length.
According to the API docs on the parallel C++ method, the maximum pulse length is 0.0016 seconds.
https://github.com/wpilibsuite/allwpilib/blob/faa29d596c9a01447a36fd5eaa18e8ad28434ad4/wpilibc/src/main/native/include/frc/DigitalOutput.h#L78-L86
Perhaps this is worthy of adding a range check and warning?
The actual maximum depends on the PWM loop timing rate, which is settable.
Thanks, we were able to get our 20ms pulse after setting the PWM Rate to 10000.
It turns out the pulse API isn't affected by the PWM rate. Its just a microsecond pulse. Just opened a PR to update both the API and the docs
I saw in your PR you set a maximum length of 65535 microseconds. Would this max value not be affected by changing the PWM rate?
Its not mattering what I'm setting PWM rate to. No matter what, the same pulse length results.
Thats strange. We were able to get different pulse lengths when we changed our pwmrate. Before setting it we couldn't get any pulse but now we're able to do 20ms up to around 70 before it doesn't pulse the right length again.
The highest you'd be able to go is 0.065. Anything higher then that, and it will roll over. Thats why going above 70 ms breaks. Anything above 65.5 ms will roll over.