psMetadataItemCompare needs clarification

[David.Robbins@…]

There are a couple of issues with psMetadataItemCompare.

First of all, there is no type checking in the compare. That means that an F64
can be compared with, for example, and S32 or even a Bool. I'm not sure if this
was the intention or not. However, I've ran some tests and because of this, if
an item is an S32 with value 1 and compared with a bool item with value 'true'
then the function will return 'true'.

On the flip side, there are cases where similar or identical types with matching
values come back false (non-matching). For instance, I ran a test comparing an
F32 item with an F64 item, both with values 1.01. Despite the fact that the
name, comment, and data values were identical, the function reported that they
did not match. This is due to the way in which the values are compared,
ie,
psMetadataItemCompare.c:32 return (template->data.TEMPLATENAME == value) ? true
: false; \

If you want to compare floats or doubles with each other or themselves, it's
best to do an abs(float - double) < DBL_EPSILON
Also, if you do want the ability to compare a double with an integer, you need
to do something like this and decide what your tolerance should be. For another
example, I tried comparing an S32 of value 1 with an F64 of value 1.00001. This
returns false. I imagine that if you want the ability to compare such values,
you'd like that case to pass. Consequently, just let me know what way to go
with this if you'd like it fixed.
thanks.

[David.Robbins@…]

Sorry, I forgot to mention that there are types of psMetadataItems missing from
this function. Also, there are no error messages for passing in a NULL. Were
these things done intentionally?

[eugene]

I think most of these issues have been addressed, and this function is now working as desired. However, it would be good to double check a few points:

• does / should the INT vs FLOAT comparison return true if (int)fvalue == ivalue or if fabs(ivalue-fvalue) < tolerance?

• should the value of the tolerance (1e-6 at the moment) be set to one of the defined _EPSILON values (eg, DBL or FLOAT)?

• The SDRS should also be updated to describe the actual functionality.