Re: [B.A.T.M.A.N.] [RFC 05/10] batman-adv: add bat_metric_is_similar API function

> > > > +static bool batadv_iv_ogm_metric_is_similar(uint32_t metric, > > > > + uint32_t new_metric) > > > > +{ > > > > + return (metric - new_metric < BATADV_TQ_SIMILARITY_THRESHOLD); > > > > > > You might want to use abs(metric - new_metric) here, otherwise is_similar(a, b) output > > > might differ from is_similar(b, a). > > > > Mh..imho the name of the function is bad because this has been done on purpose. > > agreed, the function name is not really good. You could rename it to something like > "is_almost_or_better()" if you keep the current semantics, although this is not an > "easy name" either. Or rename it to "similar_or_greater()". Something like that > > Although ... > > > > The idea is that we want to see if 'b' is at least > > as good as 'a', therefore what we want to check if is b is greater than > > 'a - threshold' only. > > > > Imagine that 'b' is greater than (a + threshold), for me the function has to > > return true, because the metric b is at least as good as a, but if I introduce > > the abs() the function would return false. > > > > Example: > > > > a=190 > > b=240 > > threshold=20 > > > > a - b = -50 < 20 => b is at least as good as a! > > > > using abs: > > > > abs(a - b) = 50 < 20 => NO! b is worse than a....and this is not true. > > > > > > this situation can happen in the code because usually 'a' will represents some > > kind of current metric and this is not supposed to be the best ever (maybe we > > still have to switch to a new best). > > ... we actually compare to the biggest value (e.g. highest gateway rank, highest > bonding candidate), so I wonder if this can really happen. So adding abs() would > just make the semantics more clear without breaking the current code when we simply > replace. Although we need to check all occurences again, I'm not completely sure > that it's always compared to the greatest member. well the current code uses the semantic that I implemented in the API (IIRC) and this is why I've done so: I wanted to keep the very same behaviour. I'm also not entirely sure that we always compare to the greatest member. What you are thinking about is the compare() function taking a threshold as parameter. We can do that with the compare() API if you want, but I think we should keep this "~is_similar()" API in order to avoid behavioural changes in the code.