From 5162d0001c4aa4b7902e92cc823b3733104929ed Mon Sep 17 00:00:00 2001 From: Thad House Date: Fri, 28 Apr 2023 20:54:58 -0700 Subject: [PATCH] [hal] Fix and document addressable LED timings (#5272) HAL_SetAddressableLEDBitTiming swapped high and low timings for whatever was written to it. This fixes that bug. Additionally, the API has been updated to take high time first, and then low time. This is due to this being the physical data format, so having the API match is clearer. Additionally, update the docs with the defaults. --- .../edu/wpi/first/hal/AddressableLEDJNI.java | 2 +- hal/src/main/native/athena/AddressableLED.cpp | 12 ++++---- .../main/native/cpp/jni/AddressableLEDJNI.cpp | 6 ++-- .../main/native/include/hal/AddressableLED.h | 4 +-- hal/src/main/native/sim/AddressableLED.cpp | 4 +-- .../src/main/native/cpp/AddressableLED.cpp | 12 ++++---- .../main/native/include/frc/AddressableLED.h | 25 +++++++++------- .../edu/wpi/first/wpilibj/AddressableLED.java | 30 ++++++++++--------- 8 files changed, 50 insertions(+), 45 deletions(-) diff --git a/hal/src/main/java/edu/wpi/first/hal/AddressableLEDJNI.java b/hal/src/main/java/edu/wpi/first/hal/AddressableLEDJNI.java index 1dd80a7b46..57f4b5dc14 100644 --- a/hal/src/main/java/edu/wpi/first/hal/AddressableLEDJNI.java +++ b/hal/src/main/java/edu/wpi/first/hal/AddressableLEDJNI.java @@ -14,7 +14,7 @@ public class AddressableLEDJNI extends JNIWrapper { public static native void setData(int handle, byte[] data); public static native void setBitTiming( - int handle, int lowTime0, int highTime0, int lowTime1, int highTime1); + int handle, int highTime0, int lowTime0, int highTime1, int lowTime1); public static native void setSyncTime(int handle, int syncTime); diff --git a/hal/src/main/native/athena/AddressableLED.cpp b/hal/src/main/native/athena/AddressableLED.cpp index 5e55c93e14..57f37cbf75 100644 --- a/hal/src/main/native/athena/AddressableLED.cpp +++ b/hal/src/main/native/athena/AddressableLED.cpp @@ -206,10 +206,10 @@ void HAL_WriteAddressableLEDData(HAL_AddressableLEDHandle handle, } void HAL_SetAddressableLEDBitTiming(HAL_AddressableLEDHandle handle, - int32_t lowTime0NanoSeconds, int32_t highTime0NanoSeconds, - int32_t lowTime1NanoSeconds, + int32_t lowTime0NanoSeconds, int32_t highTime1NanoSeconds, + int32_t lowTime1NanoSeconds, int32_t* status) { auto led = addressableLEDHandles->Get(handle); if (!led) { @@ -217,10 +217,10 @@ void HAL_SetAddressableLEDBitTiming(HAL_AddressableLEDHandle handle, return; } - led->led->writeLowBitTickTiming(1, highTime0NanoSeconds / 25, status); - led->led->writeLowBitTickTiming(0, lowTime0NanoSeconds / 25, status); - led->led->writeHighBitTickTiming(1, highTime1NanoSeconds / 25, status); - led->led->writeHighBitTickTiming(0, lowTime1NanoSeconds / 25, status); + led->led->writeLowBitTickTiming(0, highTime0NanoSeconds / 25, status); + led->led->writeLowBitTickTiming(1, lowTime0NanoSeconds / 25, status); + led->led->writeHighBitTickTiming(0, highTime1NanoSeconds / 25, status); + led->led->writeHighBitTickTiming(1, lowTime1NanoSeconds / 25, status); } void HAL_SetAddressableLEDSyncTime(HAL_AddressableLEDHandle handle, diff --git a/hal/src/main/native/cpp/jni/AddressableLEDJNI.cpp b/hal/src/main/native/cpp/jni/AddressableLEDJNI.cpp index 75da176c77..6fee9ecd12 100644 --- a/hal/src/main/native/cpp/jni/AddressableLEDJNI.cpp +++ b/hal/src/main/native/cpp/jni/AddressableLEDJNI.cpp @@ -85,12 +85,12 @@ Java_edu_wpi_first_hal_AddressableLEDJNI_setData */ JNIEXPORT void JNICALL Java_edu_wpi_first_hal_AddressableLEDJNI_setBitTiming - (JNIEnv* env, jclass, jint handle, jint lowTime0, jint highTime0, - jint lowTime1, jint highTime1) + (JNIEnv* env, jclass, jint handle, jint highTime0, jint lowTime0, + jint highTime1, jint lowTime1) { int32_t status = 0; HAL_SetAddressableLEDBitTiming(static_cast(handle), - lowTime0, highTime0, lowTime1, highTime1, + highTime0, lowTime0, highTime1, lowTime1, &status); CheckStatus(env, status); } diff --git a/hal/src/main/native/include/hal/AddressableLED.h b/hal/src/main/native/include/hal/AddressableLED.h index 76749796bf..e47c96bef7 100644 --- a/hal/src/main/native/include/hal/AddressableLED.h +++ b/hal/src/main/native/include/hal/AddressableLED.h @@ -36,10 +36,10 @@ void HAL_WriteAddressableLEDData(HAL_AddressableLEDHandle handle, int32_t length, int32_t* status); void HAL_SetAddressableLEDBitTiming(HAL_AddressableLEDHandle handle, - int32_t lowTime0NanoSeconds, int32_t highTime0NanoSeconds, - int32_t lowTime1NanoSeconds, + int32_t lowTime0NanoSeconds, int32_t highTime1NanoSeconds, + int32_t lowTime1NanoSeconds, int32_t* status); void HAL_SetAddressableLEDSyncTime(HAL_AddressableLEDHandle handle, diff --git a/hal/src/main/native/sim/AddressableLED.cpp b/hal/src/main/native/sim/AddressableLED.cpp index 75a09fbb17..d9f34aad57 100644 --- a/hal/src/main/native/sim/AddressableLED.cpp +++ b/hal/src/main/native/sim/AddressableLED.cpp @@ -146,10 +146,10 @@ void HAL_WriteAddressableLEDData(HAL_AddressableLEDHandle handle, } void HAL_SetAddressableLEDBitTiming(HAL_AddressableLEDHandle handle, - int32_t lowTime0NanoSeconds, int32_t highTime0NanoSeconds, - int32_t lowTime1NanoSeconds, + int32_t lowTime0NanoSeconds, int32_t highTime1NanoSeconds, + int32_t lowTime1NanoSeconds, int32_t* status) {} void HAL_SetAddressableLEDSyncTime(HAL_AddressableLEDHandle handle, diff --git a/wpilibc/src/main/native/cpp/AddressableLED.cpp b/wpilibc/src/main/native/cpp/AddressableLED.cpp index 538dc102b6..0baae3c1c8 100644 --- a/wpilibc/src/main/native/cpp/AddressableLED.cpp +++ b/wpilibc/src/main/native/cpp/AddressableLED.cpp @@ -65,14 +65,14 @@ void AddressableLED::SetData(std::initializer_list ledData) { FRC_CheckErrorStatus(status, "Port {}", m_port); } -void AddressableLED::SetBitTiming(units::nanosecond_t lowTime0, - units::nanosecond_t highTime0, - units::nanosecond_t lowTime1, - units::nanosecond_t highTime1) { +void AddressableLED::SetBitTiming(units::nanosecond_t highTime0, + units::nanosecond_t lowTime0, + units::nanosecond_t highTime1, + units::nanosecond_t lowTime1) { int32_t status = 0; HAL_SetAddressableLEDBitTiming( - m_handle, lowTime0.to(), highTime0.to(), - lowTime1.to(), highTime1.to(), &status); + m_handle, highTime0.to(), lowTime0.to(), + highTime1.to(), lowTime1.to(), &status); FRC_CheckErrorStatus(status, "Port {}", m_port); } diff --git a/wpilibc/src/main/native/include/frc/AddressableLED.h b/wpilibc/src/main/native/include/frc/AddressableLED.h index e6adfca28c..198eb6770c 100644 --- a/wpilibc/src/main/native/include/frc/AddressableLED.h +++ b/wpilibc/src/main/native/include/frc/AddressableLED.h @@ -17,7 +17,10 @@ namespace frc { /** - * A class for driving addressable LEDs, such as WS2812s and NeoPixels. + * A class for driving addressable LEDs, such as WS2812Bs and NeoPixels. + * + * By default, the timing supports WS2812B LEDs, but is configurable using + * SetBitTiming() * *

Only 1 LED driver is currently supported by the roboRIO. */ @@ -122,25 +125,25 @@ class AddressableLED { /** * Sets the bit timing. * - *

By default, the driver is set up to drive WS2812s, so nothing needs to + *

By default, the driver is set up to drive WS2812Bs, so nothing needs to * be set for those. * - * @param lowTime0 low time for 0 bit - * @param highTime0 high time for 0 bit - * @param lowTime1 low time for 1 bit - * @param highTime1 high time for 1 bit + * @param highTime0 high time for 0 bit (default 400ns) + * @param lowTime0 low time for 0 bit (default 900ns) + * @param highTime1 high time for 1 bit (default 900ns) + * @param lowTime1 low time for 1 bit (default 600ns) */ - void SetBitTiming(units::nanosecond_t lowTime0, units::nanosecond_t highTime0, - units::nanosecond_t lowTime1, - units::nanosecond_t highTime1); + void SetBitTiming(units::nanosecond_t highTime0, units::nanosecond_t lowTime0, + units::nanosecond_t highTime1, + units::nanosecond_t lowTime1); /** * Sets the sync time. * *

The sync time is the time to hold output so LEDs enable. Default set for - * WS2812. + * WS2812B. * - * @param syncTime the sync time + * @param syncTime the sync time (default 280us) */ void SetSyncTime(units::microsecond_t syncTime); diff --git a/wpilibj/src/main/java/edu/wpi/first/wpilibj/AddressableLED.java b/wpilibj/src/main/java/edu/wpi/first/wpilibj/AddressableLED.java index 2b1f54bc13..61a2fa6560 100644 --- a/wpilibj/src/main/java/edu/wpi/first/wpilibj/AddressableLED.java +++ b/wpilibj/src/main/java/edu/wpi/first/wpilibj/AddressableLED.java @@ -10,7 +10,9 @@ import edu.wpi.first.hal.HAL; import edu.wpi.first.hal.PWMJNI; /** - * A class for driving addressable LEDs, such as WS2812s and NeoPixels. + * A class for driving addressable LEDs, such as WS2812Bs and NeoPixels. + * + *

By default, the timing supports WS2812B LEDs, but is configurable using setBitTiming() * *

Only 1 LED driver is currently supported by the roboRIO. */ @@ -67,32 +69,32 @@ public class AddressableLED implements AutoCloseable { /** * Sets the bit timing. * - *

By default, the driver is set up to drive WS2812s, so nothing needs to be set for those. + *

By default, the driver is set up to drive WS2812Bs, so nothing needs to be set for those. * - * @param lowTime0NanoSeconds low time for 0 bit - * @param highTime0NanoSeconds high time for 0 bit - * @param lowTime1NanoSeconds low time for 1 bit - * @param highTime1NanoSeconds high time for 1 bit + * @param highTime0NanoSeconds high time for 0 bit (default 400ns) + * @param lowTime0NanoSeconds low time for 0 bit (default 900ns) + * @param highTime1NanoSeconds high time for 1 bit (default 900ns) + * @param lowTime1NanoSeconds low time for 1 bit (default 600ns) */ public void setBitTiming( - int lowTime0NanoSeconds, int highTime0NanoSeconds, - int lowTime1NanoSeconds, - int highTime1NanoSeconds) { + int lowTime0NanoSeconds, + int highTime1NanoSeconds, + int lowTime1NanoSeconds) { AddressableLEDJNI.setBitTiming( m_handle, - lowTime0NanoSeconds, highTime0NanoSeconds, - lowTime1NanoSeconds, - highTime1NanoSeconds); + lowTime0NanoSeconds, + highTime1NanoSeconds, + lowTime1NanoSeconds); } /** * Sets the sync time. * - *

The sync time is the time to hold output so LEDs enable. Default set for WS2812. + *

The sync time is the time to hold output so LEDs enable. Default set for WS2812B. * - * @param syncTimeMicroSeconds the sync time + * @param syncTimeMicroSeconds the sync time (default 280us) */ public void setSyncTime(int syncTimeMicroSeconds) { AddressableLEDJNI.setSyncTime(m_handle, syncTimeMicroSeconds);