Crash when creating new Equipment

[mattiasmaahl]

So I was creating a new Equipment.

To replicate:

1. go to Equipment tab
2. right-click on the EquipmentFolder
3. select new equipment from pop-up menu
4. fill in the various details, I changed the Physics->"Mash tun Volume" and -> Batch Size to match the boiler in mind.
5. hit save, then it crashes.

Version, Brewtarget:develop (my own local build of the latest develop)

Brewtarget-log of the crash: brewtarget.log

Here's the code that Crashes: src/UiAmountWithUnits.cpp:158~167

Measurement::Amount UiAmountWithUnits::toSI() {
   // It's a coding error to call this function if we are not dealing with a physical quantity
   Q_ASSERT(std::holds_alternative<Measurement::PhysicalQuantity>(this->getFieldType()));  <-- this line!
   Q_ASSERT(this->units);

   return Measurement::qStringToSI(this->getWidgetText(),
                                   this->units->getPhysicalQuantity(),
                                   this->getForcedSystemOfMeasurement(),
                                   this->getForcedRelativeScale());
}

@matty0ung you have any idea what this could be?

[matty0ung]

I'll have a look either tonight or tomorrow. If you have the stack trace that will make the root cause obvious (I hope). If not, don't worry, as I'm sure I'll be able to reproduce on Linux. It's probably a relatively straightforward fix. (Famous last words.)

[mattiasmaahl]

Sorry, No stacktrace available. 😞

the rows in the logfile above are the last rows in the logfile before the crash. all rows before those are from startup of the app.

[matty0ung]

No worries. Will have a proper look later. (Assert on Linux creates a core file which you open with gdb to get a stack trace, but I imagine it's a bit more complicated on Windows!)

[matty0ung]

Had a quick look. I think this is is because of lineEdit_hopUtilization->toSI() in Equipment::save(). It's not meaningful because the amount is a percentage, not a measure of a physical quantity, and therefore there is no SI unit for it - hence the assert. The fix is that, for this field, we should just use it without calling toSI(). There may be one or two other fields with similar problems. Will check and do a patch tomorrow hopefully.

[matty0ung]

The short fix is in https://github.com/Brewtarget/brewtarget/pull/635. Will have a think about something that would flush these issues out at compile-time.

[mattiasmaahl]

Correct me if I'm wrong, but don't you need values stored in the variables/objects for the functions to fail? values you don't have at compile time. A test could flush out such events though? just my 2 cents of thoughts?! 😄

[matty0ung]

It depends. In principle at least, we know at compile time whether a field holds a percentage, a date, a density, a duration, etc. Sometimes Qt makes it a bit harder because it doesn't play nicely with templates, but I think we can get the info from the subclass of BtLineEdit.

[mattiasmaahl]

Ok, that's kinda cool! and your'e right of course, We know the Unit for the field ahead of time.