Bug: Wrong unit

Post here for help on using FreeCAD's graphical user interface (GUI).
Forum rules
and Helpful information
IMPORTANT: Please click here and read this first, before asking for help

Also, be nice to others! Read the FreeCAD code of conduct!
openBrain
Veteran
Posts: 9031
Joined: Fri Nov 09, 2018 5:38 pm
Contact:

Re: Bug: Wrong unit

Post by openBrain »

The more I look at it, the more it seems to happen in the parser... :?
wmayer wrote: Ping
Could you please point to parser that is used to parse quantity spin boxes ? I'm a bit at lost among all parsers. :oops:
wmayer
Founder
Posts: 20202
Joined: Thu Feb 19, 2009 10:32 am
Contact:

Re: Bug: Wrong unit

Post by wmayer »

openBrain wrote: Mon Dec 06, 2021 2:49 pm Could you please point to parser that is used to parse quantity spin boxes ? I'm a bit at lost among all parsers. :oops:
https://github.com/FreeCAD/FreeCAD/blob ... tyParser.y
https://github.com/FreeCAD/FreeCAD/blob ... tyParser.l

Generated code:
https://github.com/FreeCAD/FreeCAD/blob ... tyParser.c
https://github.com/FreeCAD/FreeCAD/blob ... ityLexer.c

In FreeCAD code this function is used:
https://github.com/FreeCAD/FreeCAD/blob ... y.cpp#L473
openBrain
Veteran
Posts: 9031
Joined: Fri Nov 09, 2018 5:38 pm
Contact:

Re: Bug: Wrong unit

Post by openBrain »

Thanks @wmayer. I finally got it and ... it's not in the lexer/parser.

I'll explain for everyone. Gonna be a bit technical but for those interested in understanding without going into code, you should be able to safely ignore function names.

* QuantitySpinBox::userInput() is called at each key stroke
* It calls QuantitySpinBoxPrivate::validate(). This function, if no unit is set by the user, will append to the value 'unitStr' (a cached value of unit)
* Then userInput() calls -- if keyboardTracking -- QuantitySpinBox::getUserValue() -- through handlePendingEmit()->updateFromCache() --. This function will change used unit according quantity value and user unit system preference (typically here for default system : https://github.com/FreeCAD/FreeCAD/blob ... pp#L53-L81). The unit used will be cached in 'unitStr'.

Now what happens when you enter for example "0.080" without unit :
* Quantity spin box is created with default "mm" unit ('unitStr' = 'mm')
* "0" => value is internally corrected as "0 mm" -- appending default 'unitStr' --, then evaluated. Being very very small, stay in "mm"
* "0." => same
* "0.0" => same
* "0.08" => value is internally corrected as "0.08 mm", then evaluated. It is < 0.1 mm, so it is internally switched to µm (internal quantity is now "80 µm", no visible by user). 'unitStr' is switched to 'µm'
* "0.080" => value is internally corrected, but as it uses 'unitStr', it leads to '0.080 µm'. It is then evaluated, and as < 0.001 mm, it is internally switched to 'nm' (now internal value is '80 nm' and 'unitStr' is 'nm')
* If user leaves out the box, value is confirmed as '80 nm'

IMO, a solution is that QuantitySpinBoxPrivate::validate() should not append 'unitStr' to unitless values, but always the base user unit ('mm' or 'in' for length, ...). @wmayer, any thought on this ?
openBrain
Veteran
Posts: 9031
Joined: Fri Nov 09, 2018 5:38 pm
Contact:

Re: Bug: Wrong unit

Post by openBrain »

wmayer wrote: Mon Dec 06, 2021 2:55 pm In FreeCAD code this function is used:
https://github.com/FreeCAD/FreeCAD/blob ... y.cpp#L473
Hi @wmayer, I ended up with this solution : https://github.com/FreeCAD/FreeCAD/pull/5232
I don't find myself really happy with it, but didn't really find something better. May you have a quick look at it and give your feeling about this ?

A complementary question : I found that when you construct a quantity with for example

Code: Select all

Quantity(1.0, "m")
this actually leads to a value of "1 mm" --there are reasons for this to happen but won't detail here--. In your opinion, is this wanted behavior or a bug (one may eventually expect to get "1 m" instead) ?
wmayer
Founder
Posts: 20202
Joined: Thu Feb 19, 2009 10:32 am
Contact:

Re: Bug: Wrong unit

Post by wmayer »

I don't find myself really happy with it, but didn't really find something better. May you have a quick look at it and give your feeling about this ?
I will have a look in the next couple of days.
A complementary question : I found that when you construct a quantity with for example
How you can do this? Neither from Python nor C++ I can see that you can create a Quantity this way.
In your opinion, is this wanted behavior or a bug (one may eventually expect to get "1 m" instead) ?
It should create "1 m" and not "1 mm".
openBrain
Veteran
Posts: 9031
Joined: Fri Nov 09, 2018 5:38 pm
Contact:

Re: Bug: Wrong unit

Post by openBrain »

wmayer wrote: Tue Dec 07, 2021 5:38 pm I will have a look in the next couple of days.
Thanks. No urgency. ;)
A complementary question : I found that when you construct a quantity with for example
How you can do this? Neither from Python nor C++ I can see that you can create a Quantity this way.
Yes you can in C++. :)
-- With a mobile now, so from my memory. Can give more later --
Quantity has a constructor Quantity(double value,
Unit unit)
and unit has a constructor Unit(QString expr) that gives an implicit conversion, so all in all you can do what I mentioned above. ;)
Actually maybe I should have been more precised, it's

Code: Select all

Quantity(1.0, QStringLiteral("m"))
but functionally the same. ;)
When you'll look at the PR, you'll see a hack because of this bug.
I will propose a fix in the coming days for this.
wmayer
Founder
Posts: 20202
Joined: Thu Feb 19, 2009 10:32 am
Contact:

Re: Bug: Wrong unit

Post by wmayer »

openBrain wrote: Tue Dec 07, 2021 5:59 pm hat gives an implicit conversion
which is disabled in FreeCAD by setting

Code: Select all

#define QT_NO_CAST_FROM_ASCII
That's why I wondered whether your snippet is now Python or C++ code.
openBrain
Veteran
Posts: 9031
Joined: Fri Nov 09, 2018 5:38 pm
Contact:

Re: Bug: Wrong unit

Post by openBrain »

wmayer wrote: Tue Dec 07, 2021 6:24 pm
openBrain wrote: Tue Dec 07, 2021 5:59 pm hat gives an implicit conversion
which is disabled in FreeCAD by setting

Code: Select all

#define QT_NO_CAST_FROM_ASCII
That's why I wondered whether your snippet is now Python or C++ code.
My mistake I wasn't precise enough first time.
Notice here I was talking of an implicit conversion from string (precisely QString) to Unit. ;)
Anyway, do we still agree that

Code: Select all

Quantity(1.0, QStringLiteral('m'))
should evaluate to '1m' and not '1mm' as is currently?
wmayer
Founder
Posts: 20202
Joined: Thu Feb 19, 2009 10:32 am
Contact:

Re: Bug: Wrong unit

Post by wmayer »

openBrain wrote: Tue Dec 07, 2021 6:49 pm Anyway, do we still agree that

Code: Select all

Quantity(1.0, QStringLiteral('m'))
should evaluate to '1m' and not '1mm' as is currently?
Yes.
openBrain
Veteran
Posts: 9031
Joined: Fri Nov 09, 2018 5:38 pm
Contact:

Re: Bug: Wrong unit

Post by openBrain »

wmayer wrote: Wed Dec 08, 2021 6:45 am Yes.
https://github.com/FreeCAD/FreeCAD/pull/5235
Again no matter of urgency there. ;)
Post Reply