From df1fe1088e348990af744e3976f945957d872b0a Mon Sep 17 00:00:00 2001 From: josesimoes Date: Thu, 22 Oct 2020 14:35:42 +0100 Subject: [PATCH] Improve GPIO code - Add CPU_GPIO_TogglePinState. - Implemented it at platform level (API exists for all platforms except ESP32 on which it was done with the usual approach read-toggle-write). - Rework code at GpioPin::Toggle to use this. - Rework code at GpioPin::Toggle and GpioPin::Write to check for callbacks and post event. - Update assembly declaration. --- src/PAL/Include/CPU_GPIO_decl.h | 11 ++++++++++ .../sys_dev_gpio_native.cpp | 2 +- src/System.Device.Gpio/sys_dev_gpio_native.h | 5 ++--- ...gpio_native_System_Device_Gpio_GpioPin.cpp | 22 +++++++++---------- .../nanoCLR/Windows.Devices.Gpio/cpu_gpio.cpp | 5 +++++ .../nanoCLR/Windows.Devices.Gpio/cpu_gpio.cpp | 9 ++++++++ 6 files changed, 39 insertions(+), 15 deletions(-) diff --git a/src/PAL/Include/CPU_GPIO_decl.h b/src/PAL/Include/CPU_GPIO_decl.h index a0e4429489..1bd98e19ac 100644 --- a/src/PAL/Include/CPU_GPIO_decl.h +++ b/src/PAL/Include/CPU_GPIO_decl.h @@ -129,6 +129,17 @@ GpioPinValue CPU_GPIO_GetPinState(GPIO_PIN Pin); // Set state of output gpio pin void CPU_GPIO_SetPinState(GPIO_PIN Pin, GpioPinValue PinState); +// +// CPU_GPIO_TogglePinState +// +// Parameters :- +// +// pinNumber +// The number of the output pin for which the state is to be toggled. +// Return Value +// +void CPU_GPIO_TogglePinState(GPIO_PIN pinNumber); + // Check if pin is already reserved // Returns true if pin is already reserved bool CPU_GPIO_PinIsBusy(GPIO_PIN Pin); diff --git a/src/System.Device.Gpio/sys_dev_gpio_native.cpp b/src/System.Device.Gpio/sys_dev_gpio_native.cpp index 32f907ecae..27fe9864e7 100644 --- a/src/System.Device.Gpio/sys_dev_gpio_native.cpp +++ b/src/System.Device.Gpio/sys_dev_gpio_native.cpp @@ -102,7 +102,7 @@ const CLR_RT_NativeAssemblyData g_CLR_AssemblyNative_System_Device_Gpio = "System.Device.Gpio", 0xB6D0ACC1, method_lookup, - { 100, 1, 0, 3 } + { 100, 1, 0, 4 } }; // clang-format on diff --git a/src/System.Device.Gpio/sys_dev_gpio_native.h b/src/System.Device.Gpio/sys_dev_gpio_native.h index cf5a9fe209..1af0fee191 100644 --- a/src/System.Device.Gpio/sys_dev_gpio_native.h +++ b/src/System.Device.Gpio/sys_dev_gpio_native.h @@ -50,9 +50,8 @@ struct Library_sys_dev_gpio_native_System_Device_Gpio_GpioPin static const int FIELD___pinMode = 3; static const int FIELD___debounceTimeout = 4; static const int FIELD___callbacks = 5; - static const int FIELD___lastOutputValue = 6; - static const int FIELD___lastInputValue = 7; - static const int FIELD___disposedValue = 8; + static const int FIELD___lastInputValue = 6; + static const int FIELD___disposedValue = 7; NANOCLR_NATIVE_DECLARE(Read___SystemDeviceGpioPinValue); NANOCLR_NATIVE_DECLARE(Toggle___VOID); diff --git a/src/System.Device.Gpio/sys_dev_gpio_native_System_Device_Gpio_GpioPin.cpp b/src/System.Device.Gpio/sys_dev_gpio_native_System_Device_Gpio_GpioPin.cpp index b035e80739..207ffe4c7a 100644 --- a/src/System.Device.Gpio/sys_dev_gpio_native_System_Device_Gpio_GpioPin.cpp +++ b/src/System.Device.Gpio/sys_dev_gpio_native_System_Device_Gpio_GpioPin.cpp @@ -55,16 +55,13 @@ HRESULT Library_sys_dev_gpio_native_System_Device_Gpio_GpioPin::Toggle___VOID(CL // sanity check for drive mode set to output so we don't mess up writing to an input pin if (driveMode >= GpioPinDriveMode_Output) { - // Not all lower level API offer a 'toggle', so need to rely on the last output value field and toggle that - // one - GpioPinValue newState = - (GpioPinValue)(GpioPinValue_High ^ (GpioPinValue)pThis[FIELD___lastOutputValue].NumericByRef().s4); + CPU_GPIO_TogglePinState(pinNumber); - // ...write back to the GPIO... - CPU_GPIO_SetPinState(pinNumber, newState); - - // ... and finally store it - pThis[FIELD___lastOutputValue].NumericByRef().s4 = newState; + // fire event, only if there are callbacks registered + if (pThis[FIELD___callbacks].Dereference() != NULL) + { + PostManagedEvent(EVENT_GPIO, 0, (uint16_t)pinNumber, (uint32_t)CPU_GPIO_GetPinState(pinNumber)); + } } } NANOCLR_NOCLEANUP(); @@ -282,8 +279,11 @@ HRESULT Library_sys_dev_gpio_native_System_Device_Gpio_GpioPin::Write(CLR_RT_Hea { CPU_GPIO_SetPinState(pinNumber, (GpioPinValue)pinValue); - // store the output value in the field - gpioPin[FIELD___lastOutputValue].NumericByRef().s4 = pinValue; + // fire event if there are callbacks registered + if (gpioPin[FIELD___callbacks].Dereference() != NULL) + { + PostManagedEvent(EVENT_GPIO, 0, (uint16_t)pinNumber, (uint32_t)pinValue); + } } else { diff --git a/targets/CMSIS-OS/ChibiOS/nanoCLR/Windows.Devices.Gpio/cpu_gpio.cpp b/targets/CMSIS-OS/ChibiOS/nanoCLR/Windows.Devices.Gpio/cpu_gpio.cpp index a28655cce3..2c0067ab19 100644 --- a/targets/CMSIS-OS/ChibiOS/nanoCLR/Windows.Devices.Gpio/cpu_gpio.cpp +++ b/targets/CMSIS-OS/ChibiOS/nanoCLR/Windows.Devices.Gpio/cpu_gpio.cpp @@ -244,6 +244,11 @@ void CPU_GPIO_SetPinState(GPIO_PIN pin, GpioPinValue PinState) palWriteLine(GetIoLine(pin), (int)PinState); } +void CPU_GPIO_TogglePinState(GPIO_PIN pinNumber) +{ + palToggleLine(GetIoLine(pinNumber)); +} + bool CPU_GPIO_EnableInputPin( GPIO_PIN pinNumber, CLR_UINT64 debounceTimeMilliseconds, diff --git a/targets/FreeRTOS_ESP32/ESP32_WROOM_32/nanoCLR/Windows.Devices.Gpio/cpu_gpio.cpp b/targets/FreeRTOS_ESP32/ESP32_WROOM_32/nanoCLR/Windows.Devices.Gpio/cpu_gpio.cpp index 8fd746e701..1e550c5a8f 100644 --- a/targets/FreeRTOS_ESP32/ESP32_WROOM_32/nanoCLR/Windows.Devices.Gpio/cpu_gpio.cpp +++ b/targets/FreeRTOS_ESP32/ESP32_WROOM_32/nanoCLR/Windows.Devices.Gpio/cpu_gpio.cpp @@ -213,6 +213,15 @@ void CPU_GPIO_SetPinState(GPIO_PIN pin, GpioPinValue PinState) gpio_set_level((gpio_num_t)pin, (uint32_t)PinState); } +// Toggle pin state +void CPU_GPIO_TogglePinState(GPIO_PIN pinNumber) +{ + // platform DOES NOT support toggle + // need to do it "the hard way" + GpioPinValue newState = (GpioPinValue)(gpio_get_level((gpio_num_t)pinNumber) ^ GpioPinValue_High); + gpio_set_level((gpio_num_t)pinNumber, (uint32_t)newState); +} + // ISR called by IDF static void gpio_isr(void *arg) {