From 2945b9cfbeea12a8c5253f58e0cc849cf78ea175 Mon Sep 17 00:00:00 2001 From: Tom Fifield Date: Tue, 29 Oct 2024 21:41:21 +1100 Subject: [PATCH] De-duplicate Ambient LED management code (#5156) We currently have 4 different places where we have the logic for modifying LED state of the various types of Ambient LEDs, ExternalNotificationModule::SetExternalOff ExternalNotificationModule::SetExternalOn AmbientLightingThread::setLighting AmbientLightingThread::setLightingOff This patch de-duplicates the methods in ExternalNotification to a single method, using a boolean to toggle whether we're turning things on or off. --- src/modules/ExternalNotificationModule.cpp | 106 +++++++-------------- src/modules/ExternalNotificationModule.h | 3 +- 2 files changed, 33 insertions(+), 76 deletions(-) diff --git a/src/modules/ExternalNotificationModule.cpp b/src/modules/ExternalNotificationModule.cpp index 87387f0a49..41ad0ea654 100644 --- a/src/modules/ExternalNotificationModule.cpp +++ b/src/modules/ExternalNotificationModule.cpp @@ -93,7 +93,7 @@ int32_t ExternalNotificationModule::runOnce() nagCycleCutoff = UINT32_MAX; LOG_INFO("Turning off external notification: "); for (int i = 0; i < 3; i++) { - setExternalOff(i); + setExternalState(i, false); externalTurnedOn[i] = 0; LOG_INFO("%d ", i); } @@ -114,17 +114,17 @@ int32_t ExternalNotificationModule::runOnce() if (externalTurnedOn[0] + (moduleConfig.external_notification.output_ms ? moduleConfig.external_notification.output_ms : EXT_NOTIFICATION_MODULE_OUTPUT_MS) < millis()) { - getExternal(0) ? setExternalOff(0) : setExternalOn(0); + setExternalState(0, !getExternal(0)); } if (externalTurnedOn[1] + (moduleConfig.external_notification.output_ms ? moduleConfig.external_notification.output_ms : EXT_NOTIFICATION_MODULE_OUTPUT_MS) < millis()) { - getExternal(1) ? setExternalOff(1) : setExternalOn(1); + setExternalState(0, !getExternal(1)); } if (externalTurnedOn[2] + (moduleConfig.external_notification.output_ms ? moduleConfig.external_notification.output_ms : EXT_NOTIFICATION_MODULE_OUTPUT_MS) < millis()) { - getExternal(2) ? setExternalOff(2) : setExternalOn(2); + setExternalState(0, !getExternal(2)); } #if defined(HAS_NCP5623) || defined(RGBLED_RED) || defined(HAS_NEOPIXEL) || defined(UNPHONE) red = (colorState & 4) ? brightnessValues[brightnessIndex] : 0; // Red enabled on colorState = 4,5,6,7 @@ -203,86 +203,42 @@ bool ExternalNotificationModule::wantPacket(const meshtastic_MeshPacket *p) } /** - * Sets the external notification on for the specified index. + * Sets the external notification for the specified index. * - * @param index The index of the external notification to turn on. + * @param index The index of the external notification to change state. + * @param on Whether we are turning things on (true) or off (false). */ -void ExternalNotificationModule::setExternalOn(uint8_t index) +void ExternalNotificationModule::setExternalState(uint8_t index, bool on) { - externalCurrentState[index] = 1; + externalCurrentState[index] = on; externalTurnedOn[index] = millis(); switch (index) { case 1: #ifdef UNPHONE - unphone.vibe(true); // the unPhone's vibration motor is on a i2c GPIO expander + unphone.vibe(on); // the unPhone's vibration motor is on a i2c GPIO expander #endif if (moduleConfig.external_notification.output_vibra) - digitalWrite(moduleConfig.external_notification.output_vibra, true); + digitalWrite(moduleConfig.external_notification.output_vibra, on); break; case 2: if (moduleConfig.external_notification.output_buzzer) - digitalWrite(moduleConfig.external_notification.output_buzzer, true); + digitalWrite(moduleConfig.external_notification.output_buzzer, on); break; default: if (output > 0) - digitalWrite(output, (moduleConfig.external_notification.active ? true : false)); + digitalWrite(output, (moduleConfig.external_notification.active ? on : !on)); break; } -#ifdef HAS_NCP5623 - if (rgb_found.type == ScanI2C::NCP5623) { - rgb.setColor(red, green, blue); +#if defined(HAS_NCP5623) || defined(RGBLED_RED) || defined(HAS_NEOPIXEL) || defined(UNPHONE) + if (!on) { + red = 0; + green = 0; + blue = 0; } #endif -#ifdef RGBLED_CA - analogWrite(RGBLED_RED, 255 - red); // CA type needs reverse logic - analogWrite(RGBLED_GREEN, 255 - green); - analogWrite(RGBLED_BLUE, 255 - blue); -#elif defined(RGBLED_RED) - analogWrite(RGBLED_RED, red); - analogWrite(RGBLED_GREEN, green); - analogWrite(RGBLED_BLUE, blue); -#endif -#ifdef HAS_NEOPIXEL - pixels.fill(pixels.Color(red, green, blue), 0, NEOPIXEL_COUNT); - pixels.show(); -#endif -#ifdef UNPHONE - unphone.rgb(red, green, blue); -#endif -#ifdef T_WATCH_S3 - drv.go(); -#endif -} -void ExternalNotificationModule::setExternalOff(uint8_t index) -{ - externalCurrentState[index] = 0; - externalTurnedOn[index] = millis(); - - switch (index) { - case 1: -#ifdef UNPHONE - unphone.vibe(false); // the unPhone's vibration motor is on a i2c GPIO expander -#endif - if (moduleConfig.external_notification.output_vibra) - digitalWrite(moduleConfig.external_notification.output_vibra, false); - break; - case 2: - if (moduleConfig.external_notification.output_buzzer) - digitalWrite(moduleConfig.external_notification.output_buzzer, false); - break; - default: - if (output > 0) - digitalWrite(output, (moduleConfig.external_notification.active ? false : true)); - break; - } - -#if defined(HAS_NCP5623) || defined(RGBLED_RED) || defined(HAS_NEOPIXEL) || defined(UNPHONE) - red = 0; - green = 0; - blue = 0; #ifdef HAS_NCP5623 if (rgb_found.type == ScanI2C::NCP5623) { rgb.setColor(red, green, blue); @@ -304,10 +260,12 @@ void ExternalNotificationModule::setExternalOff(uint8_t index) #ifdef UNPHONE unphone.rgb(red, green, blue); #endif -#endif - #ifdef T_WATCH_S3 - drv.stop(); + if (on) { + drv.go(); + } else { + drv.stop(); + } #endif } @@ -379,19 +337,19 @@ ExternalNotificationModule::ExternalNotificationModule() LOG_INFO("Using Pin %i in digital mode", output); pinMode(output, OUTPUT); } - setExternalOff(0); + setExternalState(0, false); externalTurnedOn[0] = 0; if (moduleConfig.external_notification.output_vibra) { LOG_INFO("Using Pin %i for vibra motor", moduleConfig.external_notification.output_vibra); pinMode(moduleConfig.external_notification.output_vibra, OUTPUT); - setExternalOff(1); + setExternalState(1, false); externalTurnedOn[1] = 0; } if (moduleConfig.external_notification.output_buzzer) { if (!moduleConfig.external_notification.use_pwm) { LOG_INFO("Using Pin %i for buzzer", moduleConfig.external_notification.output_buzzer); pinMode(moduleConfig.external_notification.output_buzzer, OUTPUT); - setExternalOff(2); + setExternalState(2, false); externalTurnedOn[2] = 0; } else { config.device.buzzer_gpio = config.device.buzzer_gpio ? config.device.buzzer_gpio : PIN_BUZZER; @@ -449,7 +407,7 @@ ProcessMessage ExternalNotificationModule::handleReceived(const meshtastic_MeshP if (containsBell) { LOG_INFO("externalNotificationModule - Notification Bell"); isNagging = true; - setExternalOn(0); + setExternalState(0, true); if (moduleConfig.external_notification.nag_timeout) { nagCycleCutoff = millis() + moduleConfig.external_notification.nag_timeout * 1000; } else { @@ -462,7 +420,7 @@ ProcessMessage ExternalNotificationModule::handleReceived(const meshtastic_MeshP if (containsBell) { LOG_INFO("externalNotificationModule - Notification Bell (Vibra)"); isNagging = true; - setExternalOn(1); + setExternalState(1, true); if (moduleConfig.external_notification.nag_timeout) { nagCycleCutoff = millis() + moduleConfig.external_notification.nag_timeout * 1000; } else { @@ -476,7 +434,7 @@ ProcessMessage ExternalNotificationModule::handleReceived(const meshtastic_MeshP LOG_INFO("externalNotificationModule - Notification Bell (Buzzer)"); isNagging = true; if (!moduleConfig.external_notification.use_pwm) { - setExternalOn(2); + setExternalState(2, true); } else { #ifdef HAS_I2S audioThread->beginRttl(rtttlConfig.ringtone, strlen_P(rtttlConfig.ringtone)); @@ -495,7 +453,7 @@ ProcessMessage ExternalNotificationModule::handleReceived(const meshtastic_MeshP if (moduleConfig.external_notification.alert_message) { LOG_INFO("externalNotificationModule - Notification Module"); isNagging = true; - setExternalOn(0); + setExternalState(0, true); if (moduleConfig.external_notification.nag_timeout) { nagCycleCutoff = millis() + moduleConfig.external_notification.nag_timeout * 1000; } else { @@ -506,7 +464,7 @@ ProcessMessage ExternalNotificationModule::handleReceived(const meshtastic_MeshP if (moduleConfig.external_notification.alert_message_vibra) { LOG_INFO("externalNotificationModule - Notification Module (Vibra)"); isNagging = true; - setExternalOn(1); + setExternalState(1, true); if (moduleConfig.external_notification.nag_timeout) { nagCycleCutoff = millis() + moduleConfig.external_notification.nag_timeout * 1000; } else { @@ -518,7 +476,7 @@ ProcessMessage ExternalNotificationModule::handleReceived(const meshtastic_MeshP LOG_INFO("externalNotificationModule - Notification Module (Buzzer)"); isNagging = true; if (!moduleConfig.external_notification.use_pwm && !moduleConfig.external_notification.use_i2s_as_buzzer) { - setExternalOn(2); + setExternalState(2, true); } else { #ifdef HAS_I2S if (moduleConfig.external_notification.use_i2s_as_buzzer) { diff --git a/src/modules/ExternalNotificationModule.h b/src/modules/ExternalNotificationModule.h index 20d02d68b6..841ca6de9a 100644 --- a/src/modules/ExternalNotificationModule.h +++ b/src/modules/ExternalNotificationModule.h @@ -34,8 +34,7 @@ class ExternalNotificationModule : public SinglePortModule, private concurrency: uint32_t nagCycleCutoff = 1; - void setExternalOn(uint8_t index = 0); - void setExternalOff(uint8_t index = 0); + void setExternalState(uint8_t index = 0, bool on = false); bool getExternal(uint8_t index = 0); void setMute(bool mute) { isMuted = mute; }