Skip to content

Commit

Permalink
Fix code review comments - Changed temperature types to int16_t (sign…
Browse files Browse the repository at this point in the history
…ed). Avoided divide by 0.
  • Loading branch information
jamesharrow committed Aug 29, 2024
1 parent 4820a7e commit 7c549c3
Show file tree
Hide file tree
Showing 2 changed files with 25 additions and 19 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -152,14 +152,14 @@ class WaterHeaterManagementDelegate : public WaterHeaterManagement::Delegate
*
* @param waterTemperature The water temperature in 100th's Celsius
*/
void SetWaterTemperature(uint16_t waterTemperature);
void SetWaterTemperature(int16_t waterTemperature);

/**
* @brief Set the target water temperature of the tank
*
* @param targetWaterTemperature The water temperature in 100th's Celsius
*/
void SetTargetWaterTemperature(uint16_t targetWaterTemperature);
void SetTargetWaterTemperature(int16_t targetWaterTemperature);

/**
* @brief Determine whether the heating sources need to be turned on or off or left unchanged.
Expand Down Expand Up @@ -197,15 +197,15 @@ class WaterHeaterManagementDelegate : public WaterHeaterManagement::Delegate
* @param replacedWaterTemperature The temperature of the
* percentageReplaced water.
*/
void DrawOffHotWater(Percent percentageReplaced, uint16_t replacedWaterTemperature);
void DrawOffHotWater(Percent percentageReplaced, int16_t replacedWaterTemperature);

/**
* Set the temperature of the cold water that fills the tank as the hot water
* is drawn off.
*
* @param coldWaterTemperature The cold water temperature in 100th of a C
*/
void SetColdWaterTemperature(uint16_t coldWaterTemperature);
void SetColdWaterTemperature(int16_t coldWaterTemperature);

private:
/**
Expand All @@ -215,7 +215,7 @@ class WaterHeaterManagementDelegate : public WaterHeaterManagement::Delegate
*
* @return the target temperature
*/
uint16_t GetActiveTargetWaterTemperature() const;
int16_t GetActiveTargetWaterTemperature() const;

/**
* @brief Calculate the percentage of the water in the tank at the target
Expand Down Expand Up @@ -251,13 +251,13 @@ class WaterHeaterManagementDelegate : public WaterHeaterManagement::Delegate
WhmManufacturer * mpWhmManufacturer;

// Target water temperature in 100ths of a C
uint16_t mTargetWaterTemperature;
int16_t mTargetWaterTemperature;

// Actual water temperature in 100ths of a C
uint16_t mWaterTemperature;
int16_t mWaterTemperature;

// The cold water temperature in 100ths of a C
uint16_t mColdWaterTemperature;
int16_t mColdWaterTemperature;

// Boost command parameters

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -300,37 +300,43 @@ Status WaterHeaterManagementDelegate::HandleCancelBoost()
*
*********************************************************************************/

uint16_t WaterHeaterManagementDelegate::GetActiveTargetWaterTemperature() const
int16_t WaterHeaterManagementDelegate::GetActiveTargetWaterTemperature() const
{
// Determine the target temperature. If a boost command is in progress and has a mBoostTemporarySetpoint value use that as the
// target temperature.
// Note, in practise the actual heating is likely to be controlled by the thermostat's occupiedHeatingSetpoint most of the
// time, and the TemporarySetpoint (if not null) would be overiding the thermostat's occupiedHeatingSetpoint.
// However, this code doesn't rely upon the thermostat cluster.
uint16_t targetTemperature = (mBoostState == BoostStateEnum::kActive && mBoostTemporarySetpoint.HasValue())
? static_cast<uint16_t>(mBoostTemporarySetpoint.Value())
int16_t targetTemperature = (mBoostState == BoostStateEnum::kActive && mBoostTemporarySetpoint.HasValue())
? static_cast<int16_t>(mBoostTemporarySetpoint.Value())
: mTargetWaterTemperature;

return targetTemperature;
}

uint8_t WaterHeaterManagementDelegate::CalculateTankPercentage() const
{
int32_t tankPercentage = 100 * (static_cast<int32_t>(mWaterTemperature) - static_cast<int32_t>(mColdWaterTemperature)) /
(static_cast<int32_t>(GetActiveTargetWaterTemperature()) - static_cast<int32_t>(mColdWaterTemperature));
int32_t tankPercentage;
int32_t divisor = static_cast<int32_t>(GetActiveTargetWaterTemperature()) - static_cast<int32_t>(mColdWaterTemperature);

tankPercentage = 100;
if (divisor >= 0)
{
tankPercentage = 100 * (static_cast<int32_t>(mWaterTemperature) - static_cast<int32_t>(mColdWaterTemperature)) / divisor;
}

tankPercentage = std::min(tankPercentage, static_cast<int32_t>(100));
tankPercentage = std::max(tankPercentage, static_cast<int32_t>(0));

return static_cast<uint8_t>(tankPercentage);
}

void WaterHeaterManagementDelegate::SetColdWaterTemperature(uint16_t coldWaterTemperature)
void WaterHeaterManagementDelegate::SetColdWaterTemperature(int16_t coldWaterTemperature)
{
mColdWaterTemperature = coldWaterTemperature;
}

void WaterHeaterManagementDelegate::SetWaterTemperature(uint16_t waterTemperature)
void WaterHeaterManagementDelegate::SetWaterTemperature(int16_t waterTemperature)
{
mWaterTemperature = waterTemperature;

Expand All @@ -344,18 +350,18 @@ void WaterHeaterManagementDelegate::SetWaterTemperature(uint16_t waterTemperatur
ChangeHeatingIfNecessary();
}

void WaterHeaterManagementDelegate::SetTargetWaterTemperature(uint16_t targetWaterTemperature)
void WaterHeaterManagementDelegate::SetTargetWaterTemperature(int16_t targetWaterTemperature)
{
mTargetWaterTemperature = targetWaterTemperature;

// See if the heat needs to be turned on or off
ChangeHeatingIfNecessary();
}

void WaterHeaterManagementDelegate::DrawOffHotWater(Percent percentageReplaced, uint16_t replacedWaterTemperature)
void WaterHeaterManagementDelegate::DrawOffHotWater(Percent percentageReplaced, int16_t replacedWaterTemperature)
{
// First calculate the new average water temperature
mWaterTemperature = static_cast<uint16_t>(
mWaterTemperature = static_cast<int16_t>(
(mWaterTemperature * (100 - percentageReplaced) + replacedWaterTemperature * percentageReplaced) / 100);

// Replaces percentageReplaced% of the water in the tank with water of a temperature replacedWaterTemperature
Expand All @@ -370,7 +376,7 @@ void WaterHeaterManagementDelegate::DrawOffHotWater(Percent percentageReplaced,

bool WaterHeaterManagementDelegate::HasWaterTemperatureReachedTarget() const
{
uint16_t targetTemperature = GetActiveTargetWaterTemperature();
int16_t targetTemperature = GetActiveTargetWaterTemperature();

if (mBoostState == BoostStateEnum::kActive)
{
Expand Down

0 comments on commit 7c549c3

Please sign in to comment.