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

added new field rangeUnit in StateDescription #4

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

iamAden
Copy link
Collaborator

@iamAden iamAden commented Nov 17, 2024

Addressed Issue (1 mark):

Thermostat configurations had ambiguous min, max, and step values without a clear unit, causing issues when switching between units like Celsius and Fahrenheit. (Issue openhab#4432)

What You Have Reengineered (1.5 marks):

  • StateDescription Class: Added a rangeUnit attribute to make unit handling explicit.
  • Tests and Supporting Classes: Updated related test cases and builder classes to incorporate and validate the rangeUnit.
  • Implementation Files: Adjusted StateDescriptionFragment and its builder classes to recognize and use rangeUnit.

Impact of Changes (1 mark):

The updates make thermostat settings clearer and ensure they work correctly across different units. Existing setups stay the same if rangeUnit isn’t used, so nothing breaks.

Reengineering Strategy or Approach Used (1.5 marks):

A step-by-step refactoring approach was used to add rangeUnit without breaking existing code. Tests were updated to confirm everything functions smoothly, keeping things backward-compatible and ready for future enhancements.

@iamAden
Copy link
Collaborator Author

iamAden commented Nov 18, 2024

@suhadaudd11

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

Successfully merging this pull request may close these issues.

2 participants