Merge pull request #888 from matty0ung/usability

Fix missing signal for new recipe addition

CHANGES.markdown

@@ -23,9 +23,11 @@ Bug fixes and minor enhancements.
 * Crash on Windows when opening Print and Print Preview dialog [873](https://github.com/Brewtarget/brewtarget/issues/873)
 * Crash when you click Yes or No in the pop-up about downloading the latest version [878](https://github.com/Brewtarget/brewtarget/issues/878)
 * Drop down menu for Style and Equipment are so narrow, cannot see contents [879](https://github.com/Brewtarget/brewtarget/issues/879)
+* OG doesn't immediately change when adding malt to a recipe [880](https://github.com/Brewtarget/brewtarget/issues/880)
+* IBU doesn't update when adding hops [881](https://github.com/Brewtarget/brewtarget/issues/881)
 
 ### Release Timestamp
-Sun, 17 Nov 2024 04:00:11 +0100
+Tue, 19 Nov 2024 04:00:11 +0100
 
 ## v4.0.10
 Bug fixes and minor enhancements.

src/Logging.cpp

@@ -20,6 +20,7 @@
 
 #include <sstream>      // For std::ostringstream
 
+//#include <stacktrace>
 #include <boost/stacktrace.hpp>
 
 #include <QApplication>
@@ -527,11 +528,17 @@ QString Logging::getStackTrace() {
    //
    // TBD: Once all our compilers have full C++23 support, we should look at switching to <stacktrace> in the standard
    //      library.
+   //      As at 2024-11-19, according to https://en.cppreference.com/w/cpp/compiler_support, Clang and Apple Clang do
+   //      not have support for <stacktrace> (and GCC needs to be version 14 to have support enabled by default.
    //
    std::ostringstream stacktrace;
    stacktrace << boost::stacktrace::stacktrace();
    QString returnValue;
    QTextStream returnValueAsStream(&returnValue);
+   // What we'd like to be able to write:
+//   returnValueAsStream << "\nStacktrace:\n" << QString::fromStdString( std::to_string(std::stacktrace::current()));
+   // What we use in the meantime:
    returnValueAsStream << "\nStacktrace:\n" << QString::fromStdString(stacktrace.str());
+
    return returnValue;
 }

src/MainWindow.cpp

@@ -1895,14 +1895,15 @@ void MainWindow::showChanges(QMetaProperty* prop) {
    if (prop) {
       propName = prop->name();
    }
+   qDebug() << Q_FUNC_INFO << "propName:" << propName;
 
    // May St. Stevens preserve me
-   this->lineEdit_name      ->setText  (this->pimpl->m_recipeObs->name          ());
+   this->lineEdit_name      ->setText    (this->pimpl->m_recipeObs->name          ());
    this->lineEdit_batchSize ->setQuantity(this->pimpl->m_recipeObs->batchSize_l   ());
+   this->lineEdit_efficiency->setQuantity(this->pimpl->m_recipeObs->efficiency_pct());
    // TODO: One day we'll want to do some work to properly handle no-boil recipes....
    std::optional<double> const boilSize = this->pimpl->m_recipeObs->boil() ? this->pimpl->m_recipeObs->boil()->preBoilSize_l() : std::nullopt;
    this->lineEdit_boilSize  ->setQuantity(boilSize);
-   this->lineEdit_efficiency->setQuantity(this->pimpl->m_recipeObs->efficiency_pct());
    this->lineEdit_boilTime  ->setQuantity(this->pimpl->m_recipeObs->boil()->boilTime_mins());
    this->lineEdit_boilSg    ->setQuantity(this->pimpl->m_recipeObs->boilGrav());
    this->lineEdit_name      ->setCursorPosition(0);

src/measurement/IbuMethods.cpp

@@ -39,10 +39,10 @@ namespace {
     * \brief This intermediate calculation is used in Tinseth's formula and the mIBU formula
     *
     * \param wortGravity_sg
-    * \param boilTime_minutes usually measured from the point at which hops are added until flameout
+    * \param timeInBoil_minutes usually measured from the point at which hops are added until flameout
     */
    double calculateDecimalAlphaAcidUtilization(double const wortGravity_sg,
-                                               double const boilTime_minutes) {
+                                               double const timeInBoil_minutes) {
       //
       // TODO This is Tinseth's "Utilization Table" from which we could probably get a better value for
       //      decimalAlphaAcidUtilization via look-up and interpolation.
@@ -83,7 +83,7 @@ namespace {
       //
       // This is the short-cut way to get decimalAlphaAcidUtilization
       //
-      double const boilTimeFactor = (1.0 - exp(-0.04 * boilTime_minutes)) / 4.15;
+      double const boilTimeFactor = (1.0 - exp(-0.04 * timeInBoil_minutes)) / 4.15;
       double const bignessFactor  = 1.65 * pow(0.000125, (wortGravity_sg - 1.0));
       double const decimalAlphaAcidUtilization = bignessFactor * boilTimeFactor;
       return decimalAlphaAcidUtilization;
@@ -95,13 +95,13 @@ namespace {
    double tinseth(IbuMethods::IbuCalculationParms const & parms) {
       double const mgPerLiterOfAddedAlphaAcids = (parms.AArating * parms.hops_grams * 1000) / parms.postBoilVolume_liters;
       double const decimalAlphaAcidUtilization = calculateDecimalAlphaAcidUtilization(parms.wortGravity_sg,
-                                                                                      parms.boilTime_minutes);
+                                                                                      parms.timeInBoil_minutes);
       return decimalAlphaAcidUtilization * mgPerLiterOfAddedAlphaAcids;
-///      return ((AArating * hops_grams * 1000) / postBoilVolume_liters) * ((1.0 - exp(-0.04 * boilTime_minutes)) / 4.15) * (1.65 * pow(0.000125, (wortGravity_sg - 1)));
+///      return ((AArating * hops_grams * 1000) / postBoilVolume_liters) * ((1.0 - exp(-0.04 * timeInBoil_minutes)) / 4.15) * (1.65 * pow(0.000125, (wortGravity_sg - 1)));
    }
 
    double rager(IbuMethods::IbuCalculationParms const & parms) {
-      double const utilization = (18.11 + 13.86 * tanh((parms.boilTime_minutes - 31.32) / 18.17)) / 100.0;
+      double const utilization = (18.11 + 13.86 * tanh((parms.timeInBoil_minutes - 31.32) / 18.17)) / 100.0;
 
       double const gravityFactor = (parms.wortGravity_sg > 1.050) ? (parms.wortGravity_sg - 1.050)/0.2 : 0.0;
 
@@ -116,7 +116,7 @@ namespace {
       double const hopsFactor = parms.hops_grams/ (Measurement::Units::ounces.toCanonical(1.0).quantity * 1000.0);
       static const Polynomial p(Polynomial() << 0.7000029428 << -0.08868853463 << 0.02720809386 << -0.002340415323 << 0.00009925450081 << -0.000002102006144 << 0.00000002132644293 << -0.00000000008229488217);
 
-      //using 60 boilTime_minutes as a general table
+      //using 60 minutes as a general table
       static double const utilizationFactorTable[4][2] =  {
                         {1.050, 1},
                         {1.065, 0.9286},
@@ -136,13 +136,13 @@ namespace {
          utilizationFactor = utilizationFactorTable[3][1];
       }
 
-      return(volumeFactor * ( hopsFactor * (100 * parms.AArating) * p.eval(parms.boilTime_minutes) ) * utilizationFactor);
+      return(volumeFactor * ( hopsFactor * (100 * parms.AArating) * p.eval(parms.timeInBoil_minutes) ) * utilizationFactor);
    }
 
    /**
     * \brief Intermediate step used by mIBU formula
     */
-   double computePostBoilUtilization(double const boilTime_minutes,
+   double computePostBoilUtilization(double const timeInBoil_minutes,
                                      double const wortGravity_sg,
                                      double const postBoilVolume_liters,
                                      double const coolTime_minutes,
@@ -156,11 +156,11 @@ namespace {
 
       double const integrationTime = 0.001;
       double decimalAArating = 0.0;
-      for (double time_minutes = boilTime_minutes;
-           time_minutes < boilTime_minutes + coolTime_minutes;
+      for (double time_minutes = timeInBoil_minutes;
+           time_minutes < timeInBoil_minutes + coolTime_minutes;
            time_minutes += integrationTime) {
          double const dU = -1.65 * pow(0.000125, (wortGravity_sg-1.0)) * -0.04 * exp(-0.04*time_minutes) / 4.15;
-         double const temp_degK = 53.70 * exp(-1.0 * b * (time_minutes - boilTime_minutes)) + 319.55;
+         double const temp_degK = 53.70 * exp(-1.0 * b * (time_minutes - timeInBoil_minutes)) + 319.55;
          double const degreeOfUtilization =
             // The 1.0 case accounts for nonIAA components
             (time_minutes < 5.0) ? 1.0 : 2.39*pow(10.0,11.0)*exp(-9773.0/temp_degK);
@@ -183,8 +183,8 @@ namespace {
       if (!parms.kettleInternalDiameter_cm) { qWarning() << Q_FUNC_INFO << "kettleInternalDiameter_cm not set!"; }
       if (!parms.kettleOpeningDiameter_cm ) { qWarning() << Q_FUNC_INFO << "kettleOpeningDiameter_cm  not set!"; }
       double const decimalAlphaAcidUtilization = calculateDecimalAlphaAcidUtilization(parms.wortGravity_sg,
-                                                                                      parms.boilTime_minutes);
-      double const postBoilUtilization = computePostBoilUtilization(parms.boilTime_minutes,
+                                                                                      parms.timeInBoil_minutes);
+      double const postBoilUtilization = computePostBoilUtilization(parms.timeInBoil_minutes,
                                                                     parms.wortGravity_sg,
                                                                     parms.postBoilVolume_liters,
                                                                     parms.coolTime_minutes.value_or(0.0),

src/measurement/IbuMethods.h

@@ -91,8 +91,8 @@ namespace IbuMethods {
     *        say that Tinseth himself confirmed "Post boil volume is correct" because "We are concerned with the mg/L
     *        and any portions of a liter lost post boil doesn’t affect the calculation".
     * \param wortGravity_sg in specific gravity at around 60F I guess.
-    * \param boilTime_minutes - minutes that the hops are in the boil: usually measured from the point at which hops are
-    *                           added until flameout
+    * \param timeInBoil_minutes - minutes that the hops are in the boil: usually measured from the point at which hops
+    *                             are added until flameout
     *
     * \param coolTime_minutes - (Only used in mIbu)  Time after flameout, without forced cooling.  Note per
     *        https://alchemyoverlord.wordpress.com/2015/05/12/a-modified-ibu-measurement-especially-for-late-hopping/
@@ -110,7 +110,7 @@ namespace IbuMethods {
       double hops_grams;
       double postBoilVolume_liters;
       double wortGravity_sg;
-      double boilTime_minutes;
+      double timeInBoil_minutes;
       std::optional<double> coolTime_minutes          = std::nullopt;
       std::optional<double> kettleInternalDiameter_cm = std::nullopt;
       std::optional<double> kettleOpeningDiameter_cm  = std::nullopt;

src/model/NamedEntity.cpp

@@ -460,13 +460,14 @@ void NamedEntity::prepareForPropertyChange(BtStringConst const & propertyName) {
 }
 
 void NamedEntity::propagatePropertyChange(BtStringConst const & propertyName, bool notify) const {
+   //
+   // Normally leave this log statement commented out as otherwise it can generate a lot of lines in the log files
+   //
+//   qDebug() <<
+//      Q_FUNC_INFO << "Property name" << *propertyName << "change on" << this->metaObject()->className() <<
+//      ": m_propagationAndSignalsEnabled " << (this->m_propagationAndSignalsEnabled ? "set" : "unset") << ", notify" <<
+//      (notify ? "on" : "off");
    if (!this->m_propagationAndSignalsEnabled) {
-      //
-      // Normally leave this log statement commented out as otherwise it can generate a lot of lines in the log files
-      //
-//      qDebug() <<
-//         Q_FUNC_INFO << "Not propagating" << *propertyName << "change on" << this->metaObject()->className() <<
-//         "as m_propagationAndSignalsEnabled unset";
       return;
    }
 
@@ -491,6 +492,8 @@ void NamedEntity::notifyPropertyChange(BtStringConst const & propertyName) const
    Q_ASSERT(idx >= 0);
    QMetaProperty metaProperty = this->metaObject()->property(idx);
    QVariant value = metaProperty.read(this);
+   // Normally leave this log statement commented out as otherwise it can generate a lot of lines in the log files
+//   qDebug() << Q_FUNC_INFO << this->metaObject()->className() << ":" << propertyName << "=" << value;
    emit this->changed(metaProperty, value);
 
    return;

src/model/OwnedSet.h

@@ -81,6 +81,8 @@ template <OwnedSetOptions os> concept CONCEPT_FIX_UP IsCopyable   = is_Copyable
  *        For an "unordered" set, we do not have a strict ordering of the owned items (eg two hop additions could happen
  *        at the same time).
  *
+ *        When the set changes, we emit the \c NamedEntity::changed signal on the \c owner object.
+ *
  * \param Owner class needs to inherit from \c NamedEntity
  * \param Item class also needs to inherit from \c NamedEntity, plus implement: \c ownerId, \c setOwnerId
  *        For an enumerated set, \c Item also needs to implement \c seqNum, \c setSeqNum
@@ -195,7 +197,7 @@ class OwnedSet {
     *             be.
     */
    void acceptItemChange(Item const & item, [[maybe_unused]] QMetaProperty prop, [[maybe_unused]] QVariant val) {
-      // If one of our steps changed, our pseudo properties may also change, so we need to emit some signals
+      // If one of our items changed, our pseudo properties may also change, so we need to emit some signals
       if (item.ownerId() == this->m_owner.key()) {
          emit this->m_owner.changed(this->m_owner.metaProperty(*propertyName), QVariant());
       }
@@ -415,7 +417,10 @@ class OwnedSet {
          this->m_itemIds.append(item->key());
       }
 
-      // Now we changed the size of the set, have the owner tell people about it
+      // Now we added an item to the set, we need to listen for changes to it
+      this->connectItemChangedSignal(item);
+
+      // And, now we changed the size of the set, we have the owner tell people about it
       this->emitSetChanged();
 
       return item;

src/model/Recipe.cpp

@@ -1266,6 +1266,8 @@ class Recipe::impl {
     * Emits changed(IBU). Depends on: _batchSize_l, _boilGrav, _boilVolume_l, _finalVolume_l
     */
    void recalcIBU() {
+      qDebug() << Q_FUNC_INFO << "Recalculating IBU from" << this->m_IBU;
+
       double calculatedIbu = 0.0;
 
       // Bitterness due to hops...
@@ -1275,9 +1277,11 @@ class Recipe::impl {
       this->m_ibus.clear();
       for (auto const & hopAddition : this->m_self.hopAdditions()) {
          double tmp = this->m_self.ibuFromHopAddition(*hopAddition);
+         qDebug() << Q_FUNC_INFO << *hopAddition << "gave IBU" << tmp;
          this->m_ibus.append(tmp);
          calculatedIbu += tmp;
       }
+      qDebug() << Q_FUNC_INFO << "Calculated IBU from hops" << calculatedIbu;
 
       // Bitterness due to hopped extracts...
       for (auto const & fermentableAddition : this->m_self.fermentableAdditions()) {
@@ -1291,6 +1295,7 @@ class Recipe::impl {
                fermentableAddition->fermentable()->key() << ":" << fermentableAddition->name();
          }
       }
+      qDebug() << Q_FUNC_INFO << "Calculated IBU from hops and fermentables" << calculatedIbu;
 
       if (! qFuzzyCompare(calculatedIbu, this->m_IBU)) {
          qDebug() <<
@@ -2609,7 +2614,7 @@ double Recipe::ibuFromHopAddition(RecipeAdditionHop const & hopAddition) {
       qCritical() << Q_FUNC_INFO << "Using Hop volume as weight - THIS IS PROBABLY WRONG!";
    }
    double grams = hopAddition.quantity() * 1000.0;
-   double minutes = hopAddition.addAtTime_mins().value_or(0.0);
+   double hopTimeInBoil_mins = hopAddition.addAtTime_mins().value_or(0.0);
    // Assume 100% utilization until further notice
    double hopUtilization = 1.0;
    // Assume 60 min boil until further notice
@@ -2631,12 +2636,18 @@ double Recipe::ibuFromHopAddition(RecipeAdditionHop const & hopAddition) {
       boilTime_mins = boil->boilTime_mins();
    }
 
+   qDebug() <<
+      Q_FUNC_INFO << "Equipment" << (equipment ? "set" : "not set") << ", Boil" << (boil ? "present" : "not present") <<
+      ", Hop Utilization =" << hopUtilization << ", Boil Time (Mins) =" << boilTime_mins << ", Hop Addition" <<
+      hopAddition << ", stage =" << hopAddition.stage() << ", grams =" << grams << ", hopTimeInBoil_mins = " <<
+      hopTimeInBoil_mins << ", AArating = " << AArating;
+
    IbuMethods::IbuCalculationParms parms = {
       .AArating              = AArating,
       .hops_grams            = grams,
       .postBoilVolume_liters = this->pimpl->m_finalVolumeNoLosses_l,
       .wortGravity_sg        = m_og,
-      .boilTime_minutes      = boilTime_mins,  // Seems unlikely in reality that there would be fractions of a minute
+      .timeInBoil_minutes    = boilTime_mins,  // Seems unlikely in reality that there would be fractions of a minute
       .coolTime_minutes      = boil->coolTime_mins(),
    };
    if (equipment) {
@@ -2646,13 +2657,17 @@ double Recipe::ibuFromHopAddition(RecipeAdditionHop const & hopAddition) {
    if (hopAddition.isFirstWort()) {
       ibus = fwhAdjust * IbuMethods::getIbus(parms);
    } else if (hopAddition.stage() == RecipeAddition::Stage::Boil) {
-      parms.boilTime_minutes = minutes;
+      parms.timeInBoil_minutes = hopTimeInBoil_mins;
       ibus = IbuMethods::getIbus(parms);
    } else if (hopAddition.stage() == RecipeAddition::Stage::Mash && mashHopAdjust > 0.0) {
       ibus = mashHopAdjust * IbuMethods::getIbus(parms);
+   } else {
+      qDebug() << Q_FUNC_INFO << "No IBUs from " << hopAddition;
    }
 
-   // Adjust for hopAddition form. Tinseth's table was created from whole cone data,
+   qDebug() << Q_FUNC_INFO << "IBUs before adjustment for form =" << ibus;
+
+   // Adjust for hop form. Tinseth's table was created from whole cone data,
    // and it seems other formulae are optimized that way as well. So, the
    // utilization is considered unadjusted for whole cones, and adjusted
    // up for plugs and pellets.

src/undoRedo/UndoableAddOrRemove.h

@@ -1,5 +1,5 @@
 /*╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌
- * undoRedo/UndoableAddOrRemove.h is part of Brewtarget, and is copyright the following authors 2020-2022:
+ * undoRedo/UndoableAddOrRemove.h is part of Brewtarget, and is copyright the following authors 2020-2024:
  *   • Mattias Måhl <[email protected]>
  *   • Matt Young <[email protected]>
  *
@@ -27,6 +27,7 @@
 #include <QUndoCommand>
 
 #include "database/ObjectStoreWrapper.h"
+#include "Logging.h"
 
 class MainWindow;
 
@@ -148,6 +149,8 @@ class UndoableAddOrRemove : public QUndoCommand {
             Q_FUNC_INFO << (this->everDone ? "Redo" : "Do" ) << this->text() << "for " <<
             this->whatToAddOrRemove->metaObject()->className() << "#" << this->whatToAddOrRemove->key() << "(" <<
             this->whatToAddOrRemove->name() << ")";
+         // Normally leave the next line commented out as it generates a lot of logging
+//         qDebug().noquote() << Q_FUNC_INFO << Logging::getStackTrace();
 
          this->whatToAddOrRemove = (this->updatee.*(this->doer))(this->whatToAddOrRemove);
          qDebug() <<

translations/bt_ca.ts

@@ -8484,7 +8484,7 @@ El volum final al primari és de %1.</translation>
         <translation type="unfinished"></translation>
     </message>
     <message>
-        <source>(See Boil tab below)</source>
+        <source>See Boil tab below</source>
         <translation type="unfinished"></translation>
     </message>
 </context>

translations/bt_cs.ts

@@ -8336,7 +8336,7 @@ Celkový objem pro hlavní kvašení je %1.</translation>
         <translation type="unfinished"></translation>
     </message>
     <message>
-        <source>(See Boil tab below)</source>
+        <source>See Boil tab below</source>
         <translation type="unfinished"></translation>
     </message>
 </context>