[Ovmsdev] DBC : proposal fix for negative integer numbers having wrong values

[Ludovic LANGE]

Hi Michael,

Thank you a lot for your message... however I must admit my complete 
ignorance (and lack of practice) of the whole C++ / templating world. I 
mean that I cannot understand (yet) your template version - such a shame 
; nor adapt it to this fix.

Just if you have a few minutes to enlighten me : if we were to look at 
the proposed patch here 
https://github.com/openvehicles/Open-Vehicle-Monitoring-System-3/pull/736/files 
; and if we would introduce your template implementation, how should we 
rewrite the "patched" version to make use of this implementation ?

    result.Cast((uint32_t)SIGNEX(val, m_signal_size-1),
    DBC_NUMBER_INTEGER_SIGNED);

(my naïve attempts all failed compilation with all sorts of "error: use 
of 'this' in a constant expression" and "error: no matching function for 
call to 'sign_extend(uint64_t&)'" so a little help would be appreciated)

Additionally, could you please expand a little on the benefits it adds - 
you're talking about type-safety, if you could just illustrate it in 
this context ?

Sorry for being ignorant on this subject.

Regards,

Ludovic


Le 14/10/2022 à 10:40, Michael Geddes a écrit :
> OOH.. nice one. I had missed that in my template implementation.  I'm 
> a big fan of using C++ templating to do type-safe versions of this 
> kind of stuff...   This is my template version:
> /** Sign extend an unsigned to a signed integer of the same size.
>  */
> template<typename UINT, typename INT, uint8_t SIGNBIT>
> INT sign_extend( UINT uvalue)
> {
>   typedef typename std::make_unsigned<INT>::type uint_t;
>   uint_t newuvalue = uvalue;
>   if ( newuvalue & ( UINT(1U) << SIGNBIT) ) {
>     newuvalue |= ~((uint_t(1U) << SIGNBIT) - 1);
>   }
>   return reinterpret_cast<INT &>(uvalue);
> }
> It could probably be converted into a version that would infer some of 
> the types.. but this should work pretty well and should compile down 
> to close to the same code.
> Usage from my code: Where BYTES is a templated integer value parameter.
>  res = sign_extend < uint32_t, int32_t, BYTES * 8 - 1 > (ures);
>
> I also have a templated buffer extractor that allows for checking 
> available space!
> For example this code (which extracts 2 bytes as a signed integer), 
> which should now work with sign extension thanks to your bugfix and 
> the above templated version of sign extension:
>
>          int32_t signedValue;
>          if (!can_buff_int<2>(data, 10, signedValue )) {
>            ESP_LOGE(TAG, "IoniqISOTP.BMC: BMS Current: Bad Buffer");
>          } else {
>  StdMetrics.ms_v_bat_current->SetValue((float)signedValue / 10.0, Amps);
>          }
>
> //.ichael
>
>
>
>
> On Fri, 14 Oct 2022 at 05:35, Ludovic LANGE <ll-ovmsdev at lange.nom.fr> 
> wrote:
>
>     Dear list,
>
>     I've come across an issue with the DBC parser where a non-32-bit
>     negative number was incorrectly decoded.
>
>     I've traced this to an issue with sign-extension ; and I've
>     proposed a fix here
>     https://github.com/openvehicles/Open-Vehicle-Monitoring-System-3/pull/736
>
>     Before having it merged, I'd like to gather some feedback from
>     users of the DBC module.
>
>     Please have a look, and comment directly on the GitHub PR.
>
>     Thanks in advance !
>
>     Regards,
>
>     Ludovic
>
>     _______________________________________________
>     OvmsDev mailing list
>     OvmsDev at lists.openvehicles.com
>     http://lists.openvehicles.com/mailman/listinfo/ovmsdev
>
>
> _______________________________________________
> OvmsDev mailing list
> OvmsDev at lists.openvehicles.com
> http://lists.openvehicles.com/mailman/listinfo/ovmsdev

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.openvehicles.com/pipermail/ovmsdev/attachments/20221022/75ca8169/attachment-0001.htm>