Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

De-duplicate Ambient LED management code #5194

Open
fifieldt opened this issue Oct 31, 2024 · 1 comment
Open

De-duplicate Ambient LED management code #5194

fifieldt opened this issue Oct 31, 2024 · 1 comment
Labels
good first issue Good for newcomers help wanted Extra attention is needed low-priority Possibly something we'll consider in the future but doesn't serve the core use-cases or problematic

Comments

@fifieldt
Copy link
Contributor

We currently have 3 different places where we have the logic for modifying LED state of the various types of Ambient LEDs,
ExternalNotificationModule::SetExternalState
AmbientLightingThread::setLighting
AmbientLightingThread::setLightingOff

Instead, we should have a single place where this happens.

Use the notification system we have in the firmware for IPC, so that Ambient lighting offers a subscription service for LED changes and the External Notifications uses it (if available).

You can look at how it's done for notifying others about GPS position changes :-)

@fifieldt fifieldt added help wanted Extra attention is needed good first issue Good for newcomers low-priority Possibly something we'll consider in the future but doesn't serve the core use-cases or problematic labels Oct 31, 2024
@Blake-Latchford
Copy link
Contributor

You can look at how it's done for notifying others about GPS position changes :-)

newStatus.notifyObservers(&status);

Is this what you're referring to? If I understand the proposal correctly:

  1. AmbientLightingThread should own something like an Observable<LightingConfig> lightChanged.
  2. AmbientLightingThread::setLighting is invoked by an observer.
  3. LightingConfig probably looks a lot like meshtastic_ModuleConfig_AmbientLightingConfig, but also needs to identify which light is being configured. Though maybe you're expecting that the observer is just watching for ambient lighting.
  4. AmbientLightingThread::setLightingOff calls observable.notifyObservers(&offStruct)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers help wanted Extra attention is needed low-priority Possibly something we'll consider in the future but doesn't serve the core use-cases or problematic
Projects
None yet
Development

No branches or pull requests

2 participants