ETLCPP / etl

Embedded Template Library
https://www.etlcpp.com
MIT License
2.2k stars 391 forks source link

unsupported crc16(XModem) standart #171

Closed danielklioc closed 4 years ago

danielklioc commented 4 years ago

Hello,

I am using etl in crc calculation. I use crc16_ccitt with intial value 0xFFFF. But I need support for crc16_ccitt with intial value 0x0000.

I have created copy of policy with intial value 0x0000 and can create pull request. Is it proper solution?

jwellbelove commented 4 years ago

Does this CRC variant with an initial value have a specific name i.e. CRC16 DNP, CRC16 DECT etc If so, then create a policy and frame check sequence derived class with this name.

hoo2 commented 4 years ago

I believe it is CRC16-CCITT poly:0x1021 (or 0x8408 for reverse order). It is used for X.25, V.41, HDLC FCS, XMODEM, Bluetooth, PACTOR, SD, DigRF and many others... It is already supported by the library. crc16_ccitt() is that you want.

danielklioc commented 4 years ago

@jwellbelove yes, it has specific name CRC-16/XMODEM. Is name crc16_xmodem and crc_policy_16_xmodem ok? @hoo2 crc16_ccitt initial value 0xFFFF, for xmodem 0x0000, but poly is same. This is why I need different policy.

jwellbelove commented 4 years ago

Yes, you could create a crc_policy_16_xmodem, and a crc16_xmodem derived class. If other CRC variants use exactly the same algorithm then they could maybe be typedef'd from it (though it may cause issues if you want to overload on the CRC type)

jwellbelove commented 4 years ago

Were you planning to do a pull request in the near future for this?

danielklioc commented 4 years ago

I have written another implementation with template and typedef, to have option to choose ccitt or ccitt_xmodem standart. Cannot finalize it because of compile error.

Could you help to find issue?

my changes in crc16_ccitt.h

namespace etl
{
  //***************************************************************************
  /// CRC16 CCITT policy.
  /// Calculates CRC16 CCITT using polynomial 0x1021
  //***************************************************************************
  template<uint16_t I>
  struct crc_policy_16_ccitt
  {
    typedef uint16_t value_type;

    inline uint16_t initial() const
    {
      return I;
    }
...

  //*************************************************************************
  /// CRC16 CCITT
  //*************************************************************************
  template<uint16_t I>
  class crc16_ccitt_base : public etl::frame_check_sequence<etl::crc_policy_16_ccitt<I>>
  {
  public:

    //*************************************************************************
    /// Default constructor.
    //*************************************************************************
      crc16_ccitt_base()
    {
      this->reset();
    }

    //*************************************************************************
    /// Constructor from range.
    /// \param begin Start of the range.
    /// \param end   End of the range.
    //*************************************************************************
    template<typename TIterator>
    crc16_ccitt_base(TIterator begin, const TIterator end)
    {
      this->reset();
      this->add(begin, end);
    }
  };

  typedef crc16_ccitt_base<0xFFFF> crc16_ccitt;
  typedef crc16_ccitt_base<0x0000> crc16_ccitt_xmodem;
}

#endif

but got stuck with compile error

invalid template arguments type/value mismatch at argument 1 in template parameter list for 'template struct etl::crc_policy_16_ccitt

#define PROFILE_CPP11_fGENERIC 1
#include <iostream>
#include <stdio.h>
#include <stdint.h>
#include <sys/time.h>
#include "etl_profile.h"
#include <string>

#include "crc16_ccitt.h"

using namespace std;

int main(){
std::string data("123456789");
uint16_t crc_temp = etl::crc16_ccitt_base< etl::crc_policy_16_ccitt<etl::crc16_ccitt> >( data.begin(), data.end() ); 
}
jwellbelove commented 4 years ago

You are giving _etl::crc_policy_16ccitt a the wrong type of template parameter. i.e. a etl::crc16_ccitt instead of an initial value.

jwellbelove commented 4 years ago

Do you need any more assistance with the code?

danielklioc commented 4 years ago

@jwellbelove it is not clear for me what you meant in previous comment, I do not understand how to use properly templates in this case. Would appreciate help

jwellbelove commented 4 years ago

I think the code you posted should be this... uint16_t crc_temp = etl::crc16_ccitt_base< etl::crc_policy_16_ccitt<0xFFFF> >( data.begin(), data.end() );

danielklioc commented 4 years ago

I have tried this way, but getting same errors

jwellbelove commented 4 years ago

OK I can see the problem. I was getting confused witht the multiple class templates.

etl::crc16_ccitt_base expects a uint16_t template parameter.

`etl::crc16_ccitt_base<0xFFFF>( data.begin(), data.end() );

The answer was in your typedefs typedef crc16_ccitt_base<0xFFFF> crc16_ccitt;

danielklioc commented 4 years ago

I understood mistake. Implementation works correctly for ccitt and ccitt_xmodem. Does my changes to ccitt is proper solution? Can prepare pull request?

jwellbelove commented 4 years ago

Yes, do a pull request and I will pull it into a feature branch and review it.