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

Setting lighting #811

Open
HvanderStok opened this issue Feb 16, 2025 · 0 comments
Open

Setting lighting #811

HvanderStok opened this issue Feb 16, 2025 · 0 comments

Comments

@HvanderStok
Copy link
Contributor

In the current implementation, there are multiple ways to set the lighting power:

  • When use_maintained_illuminance is set to True, the lighting power is computed as lighting_power = maintained_illuminance / lighting_efficiency_lumen.
  • When use_maintained_illuminance is False, the value of fixed_lighting_power is used.
  • There is also a direct setter for lighting_power that, when invoked, forces use_maintained_illuminance to False.

This design leads to some issues:

  • Unclear Dependencies: The relationship between lighting_power, fixed_lighting_power, maintained_illuminance, and lighting_efficiency_lumen is not obvious. Changes to maintained_illuminance, lighting_efficiency_lumen or fixed_lighting_power do not automatically update lighting_power if they are modified after use_maintained_illuminance was enabled.
  • Risk of Inconsistency: Since the lighting_power attribute is stored separately and only updated when use_maintained_illuminance is set (or via the explicit setter), there is a risk that the value passed to Modelica does not reflect the latest lighting parameters.
  • Non-Intuitive Naming: It is not clear what lighting sources correspond to the old and maintained state. Also, the parameter maintained_illuminance is somewhat misleading. A more intuitive name would be simply illuminance, indicating the target illuminance value, which should be the same for old and maintained lighting sources.
  • Redundant Parameter: The fixed_lighting_power attribute appears redundant. Instead of having two separate methods of setting the lighting power, it would be preferable to rely on a single, computed value.

Proposed Changes:

  • Rename maintained_illuminance to illuminance: Simplify the naming to clearly indicate that this parameter is the zone illuminance.
  • Remove fixed_lighting_power: Eliminate the redundant parameter and compute lighting_power dynamically.
  • Adopt a Record-Based Approach for Lighting Sources: Extend the direct setting of lighting_efficiency_lumen with a record that maps lighting source names to their corresponding efficiency (in lm/W). Users can then specify a lighting source like "Incandescent", "Halogen" "LED" or "fluorescent".
  • Adopt UseConditions.json: Map old fixed lighting powers to lighting sources and just set a default one for each zone type.
  • Remove use_maintained_illuminance

@FWuellhorst @DaJansenGit @HoeppJ what are your opinions on that and should we still allow the direct setting of the lighting_power to an area-specific value, or just have the one setting method with the efficiency?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant