CCS C Software and Maintenance Offers
FAQFAQ   FAQForum Help   FAQOfficial CCS Support   SearchSearch  RegisterRegister 

ProfileProfile   Log in to check your private messagesLog in to check your private messages   Log inLog in 

CCS does not monitor this forum on a regular basis.

Please do not post bug reports on this forum. Send them to CCS Technical Support

Table CRC vs Bit-Shifting CRC question

 
Post new topic   Reply to topic    CCS Forum Index -> General CCS C Discussion
View previous topic :: View next topic  
Author Message
RKnapp



Joined: 23 Feb 2004
Posts: 51

View user's profile Send private message

Table CRC vs Bit-Shifting CRC question
PostPosted: Tue Apr 06, 2004 8:54 pm     Reply with quote

Friends,

A while back a table-based CRCITT-16 algorithm was posted here which was clean and undoubtedly quite fast:

Code:

/* Table of CRC values for high order byte */
const char Table_CRC_Hi[256] = {
0x00, 0xC1, 0x81, 0x40, 0x01, 0xC0, 0x80, 0x41, 0x01, 0xC0, 0x80, 0x41, 0x00, 0xC1, 0x81,
0x40, 0x01, 0xC0, 0x80, 0x41, 0x00, 0xC1, 0x81, 0x40, 0x00, 0xC1, 0x81, 0x40, 0x01, 0xC0,
0x80, 0x41, 0x01, 0xC0, 0x80, 0x41, 0x00, 0xC1, 0x81, 0x40, 0x00, 0xC1, 0x81, 0x40, 0x01,
0xC0, 0x80, 0x41, 0x00, 0xC1, 0x81, 0x40, 0x01, 0xC0, 0x80, 0x41, 0x01, 0xC0, 0x80, 0x41,
0x00, 0xC1, 0x81, 0x40, 0x01, 0xC0, 0x80, 0x41, 0x00, 0xC1, 0x81, 0x40, 0x00, 0xC1, 0x81,
0x40, 0x01, 0xC0, 0x80, 0x41, 0x00, 0xC1, 0x81, 0x40, 0x01, 0xC0, 0x80, 0x41, 0x01, 0xC0,
0x80, 0x41, 0x00, 0xC1, 0x81, 0x40, 0x00, 0xC1, 0x81, 0x40, 0x01, 0xC0, 0x80, 0x41, 0x01,
0xC0, 0x80, 0x41, 0x00, 0xC1, 0x81, 0x40, 0x01, 0xC0, 0x80, 0x41, 0x00, 0xC1, 0x81, 0x40,
0x00, 0xC1, 0x81, 0x40, 0x01, 0xC0, 0x80, 0x41, 0x01, 0xC0, 0x80, 0x41, 0x00, 0xC1, 0x81,
0x40, 0x00, 0xC1, 0x81, 0x40, 0x01, 0xC0, 0x80, 0x41, 0x00, 0xC1, 0x81, 0x40, 0x01, 0xC0,
0x80, 0x41, 0x01, 0xC0, 0x80, 0x41, 0x00, 0xC1, 0x81, 0x40, 0x00, 0xC1, 0x81, 0x40, 0x01,
0xC0, 0x80, 0x41, 0x01, 0xC0, 0x80, 0x41, 0x00, 0xC1, 0x81, 0x40, 0x01, 0xC0, 0x80, 0x41,
0x00, 0xC1, 0x81, 0x40, 0x00, 0xC1, 0x81, 0x40, 0x01, 0xC0, 0x80, 0x41, 0x00, 0xC1, 0x81,
0x40, 0x01, 0xC0, 0x80, 0x41, 0x01, 0xC0, 0x80, 0x41, 0x00, 0xC1, 0x81, 0x40, 0x01, 0xC0,
0x80, 0x41, 0x00, 0xC1, 0x81, 0x40, 0x00, 0xC1, 0x81, 0x40, 0x01, 0xC0, 0x80, 0x41, 0x01,
0xC0, 0x80, 0x41, 0x00, 0xC1, 0x81, 0x40, 0x00, 0xC1, 0x81, 0x40, 0x01, 0xC0, 0x80, 0x41,
0x00, 0xC1, 0x81, 0x40, 0x01, 0xC0, 0x80, 0x41, 0x01, 0xC0, 0x80, 0x41, 0x00, 0xC1, 0x81,
0x40
} ;

/* Table of CRC values for low order byte */
const char Table_CRC_Lo[256] = {
0x00, 0xC0, 0xC1, 0x01, 0xC3, 0x03, 0x02, 0xC2, 0xC6, 0x06, 0x07, 0xC7, 0x05, 0xC5, 0xC4,
0x04, 0xCC, 0x0C, 0x0D, 0xCD, 0x0F, 0xCF, 0xCE, 0x0E, 0x0A, 0xCA, 0xCB, 0x0B, 0xC9, 0x09,
0x08, 0xC8, 0xD8, 0x18, 0x19, 0xD9, 0x1B, 0xDB, 0xDA, 0x1A, 0x1E, 0xDE, 0xDF, 0x1F, 0xDD,
0x1D, 0x1C, 0xDC, 0x14, 0xD4, 0xD5, 0x15, 0xD7, 0x17, 0x16, 0xD6, 0xD2, 0x12, 0x13, 0xD3,
0x11, 0xD1, 0xD0, 0x10, 0xF0, 0x30, 0x31, 0xF1, 0x33, 0xF3, 0xF2, 0x32, 0x36, 0xF6, 0xF7,
0x37, 0xF5, 0x35, 0x34, 0xF4, 0x3C, 0xFC, 0xFD, 0x3D, 0xFF, 0x3F, 0x3E, 0xFE, 0xFA, 0x3A,
0x3B, 0xFB, 0x39, 0xF9, 0xF8, 0x38, 0x28, 0xE8, 0xE9, 0x29, 0xEB, 0x2B, 0x2A, 0xEA, 0xEE,
0x2E, 0x2F, 0xEF, 0x2D, 0xED, 0xEC, 0x2C, 0xE4, 0x24, 0x25, 0xE5, 0x27, 0xE7, 0xE6, 0x26,
0x22, 0xE2, 0xE3, 0x23, 0xE1, 0x21, 0x20, 0xE0, 0xA0, 0x60, 0x61, 0xA1, 0x63, 0xA3, 0xA2,
0x62, 0x66, 0xA6, 0xA7, 0x67, 0xA5, 0x65, 0x64, 0xA4, 0x6C, 0xAC, 0xAD, 0x6D, 0xAF, 0x6F,
0x6E, 0xAE, 0xAA, 0x6A, 0x6B, 0xAB, 0x69, 0xA9, 0xA8, 0x68, 0x78, 0xB8, 0xB9, 0x79, 0xBB,
0x7B, 0x7A, 0xBA, 0xBE, 0x7E, 0x7F, 0xBF, 0x7D, 0xBD, 0xBC, 0x7C, 0xB4, 0x74, 0x75, 0xB5,
0x77, 0xB7, 0xB6, 0x76, 0x72, 0xB2, 0xB3, 0x73, 0xB1, 0x71, 0x70, 0xB0, 0x50, 0x90, 0x91,
0x51, 0x93, 0x53, 0x52, 0x92, 0x96, 0x56, 0x57, 0x97, 0x55, 0x95, 0x94, 0x54, 0x9C, 0x5C,
0x5D, 0x9D, 0x5F, 0x9F, 0x9E, 0x5E, 0x5A, 0x9A, 0x9B, 0x5B, 0x99, 0x59, 0x58, 0x98, 0x88,
0x48, 0x49, 0x89, 0x4B, 0x8B, 0x8A, 0x4A, 0x4E, 0x8E, 0x8F, 0x4F, 0x8D, 0x4D, 0x4C, 0x8C,
0x44, 0x84, 0x85, 0x45, 0x87, 0x47, 0x46, 0x86, 0x82, 0x42, 0x43, 0x83, 0x41, 0x81, 0x80,
0x40
};

// =============================================================================
int16 tablecrc ( char * message, int8 length )
{
   // Global Data Declarations:
   // I'll be doing this so often that the temps might as well be global: (faster)
   static char    CRC_Lo,
                  CRC_Hi;               // "static" means "global" in CCS-land
   static int8      CRC_index;            // temp

   static int16   CRCreturn;

   // Local Data Definitions:

   // Begin Procedure ==========================================================
   CRC_Hi       = 0xFF;
   CRC_Lo       = 0xFF;

   while ( length -- )
   {
      CRC_index    = CRC_Hi ^ *(message++);
      CRC_Hi       = CRC_Lo ^ Table_CRC_Hi [ CRC_index ];
      CRC_Lo       = Table_CRC_Lo [ CRC_index ];
   }

   CRCreturn      = CRC_Hi;
   CRCreturn       = (CRCreturn<<8) | CRC_Lo;
   return          CRCreturn;

}    


However, I must have a bug in here, because although it will generate a value, I don't think it's correct.

I don't think the generated value is correct because I've been using a bit-shifting technique for some time, and I *THINK* it is giving the correct answer:

Code:

#define   MTT   0x1021

int16 shiftcrc ( char * ptr, int8 length )
{
   // Global Data Declarations:
   static int8      i;
   static int16   crc;

   // Local Data Definitions:

   // Begin Procedure ==========================================================
   crc            = 0;

   while ( length -- )
   {
      crc = crc ^ ((int16)*ptr++ << 8);

      for (i=0; i<8; ++i)
         if (crc & 0x8000)
            crc = (crc << 1) ^ MTT;
         else
            crc = crc << 1;
   }

   return ( crc & 0xFFFF );

}    // end <shiftcrc>
// =============================================================================


Does anyone spot anything obvious about either algorithm?

Thanks for your attention,

Smile Robert
Neutone



Joined: 08 Sep 2003
Posts: 839
Location: Houston

View user's profile Send private message

PostPosted: Tue Apr 06, 2004 11:06 pm     Reply with quote

In general both methods work. I did some testing with some very large packets and determined the time to encode/decode at around 150 instructions per byte using the bit shift method. That was after doing all the optimizing I could. I haven't done that testing for the loolkup table routines but they looked like they would run faster. The bitshift method has much smaller program space footprint though. The routines I use are different by the polynomial factor used. The methods are the same. Have a look at the routines I posted if you like. I don't think they will help because the lookup table has to be solved for the polynomial factor.
RKnapp



Joined: 23 Feb 2004
Posts: 51

View user's profile Send private message

PostPosted: Tue Apr 06, 2004 11:13 pm     Reply with quote

Neutone,

Yiih -- 150 instructions per byte for bit shifting. Thanks for checking this out.

But both methods should compute the same answer, right? Something is funky about the table-CRC method I've posted. It appears to be quite simple so I'm wondering if:

a) There's an error in one / both of the tables -- does anyone know how to regenerate them?

b) something else quite obvious about the length of the message... something.

My application requires lots of these CRCs but I'm already up to 40% duty cycle with bit-shifting, so table-CRC seems attractive if I could make it work.

?

Thanks,

Smile Robert

ps. CCS Compiler v3.187, PIC18F8720.
guest
Guest







PostPosted: Wed Apr 07, 2004 1:59 am     Reply with quote

Well, yes, given the same input sequence, and assuming both are using the same polynomial, then of course you must expect the same result.

In the past, I've had problems with typos in the tables, and they can be devastating because everything seems to work except once in a while unexpected CRC errors are detected.

How I've dealt with that in the past is to use the bit-cycle method to build the table. This can be done "at compile time" by generating the ascii hex for the tables and then including it in a source file, or it can be done at run time during initialisation. This last idea is very good when program space is limited but RAM is plenty. In fact, by using conditional defines, any of the three methods can be selected at compile time.

In terms of speed, table lookups are by far the fastest.

cheers
Neutone



Joined: 08 Sep 2003
Posts: 839
Location: Houston

View user's profile Send private message

PostPosted: Wed Apr 07, 2004 7:43 am     Reply with quote

For what it's worth this works for me. My tables are defined much like yours.
Code:
      {  Comm1.CRC_Lo = 0xFF;                               // Prepair to generate CRC
         Comm1.CRC_Hi = 0xFF;                               // Prepair to generate CRC
         COMM1.CRClo_Index=Comm1.CRChi_Index-1;             // Solve this index once instead of once per byte
         Comm1.Index = 0;                                   // Start at begining of packet
         while(Comm1.Index < COMM1.CRClo_Index)             // Use all bytes prior to CRClo_Index
         {  Comm1.Table_Index = COMM1_Buffer[Comm1.Index];  // Generate CRC
            Comm1.Table_Index ^= Comm1.CRC_Lo;              // Generate CRC
            Comm1.CRC_Lo = Table_CRC_Hi[Comm1.Table_Index]; // Generate CRC
            Comm1.CRC_Lo ^= Comm1.CRC_Hi;                   // Generate CRC
            Comm1.CRC_Hi = Table_CRC_Lo[Comm1.Table_Index]; // Generate CRC
            Comm1.Index++;
         }
         Comm1.Index = 0;                                   // Zero index for Transmition
         COMM1_Buffer[COMM1.CRClo_Index]=Comm1.CRC_Lo;      // Place CRC_Lo within packet
         COMM1_Buffer[Comm1.CRChi_Index]=Comm1.CRC_Hi;      // Place CRC_Hi within packet
         enable_interrupts(INT_TBE);                        // Kick Off Xmit of Data
         ++Reply_Packets_This_Second;                       // Count responce packets (for debugging)
      }
PCM programmer



Joined: 06 Sep 2003
Posts: 21708

View user's profile Send private message

PostPosted: Wed Apr 07, 2004 12:08 pm     Reply with quote

Quote:
Does anyone spot anything obvious about either algorithm?


Those tables and the code to run them are all over the net.
In some cases they have the table names reversed, from
what you are showing. Do a Google search for this
string (including the quotes):
"0x00, 0xC1, 0x81, 0x40, 0x01, 0xC0, 0x80, 0x41"
If your table is defective, you can grab another one
from one of the many sites that post this code.
RKnapp



Joined: 23 Feb 2004
Posts: 51

View user's profile Send private message

PostPosted: Wed Apr 07, 2004 1:28 pm     Reply with quote

Neutone & PCM,

THANKS! Sure enough, the table-based algorithm works if one simply swaps the names on the tables -- lo to hi & vice versa. The table as posted is not damaged.

Working through Neutone's algorithm, I was beginning to wonder if the way the table-based CRC (as posted above) was processing the message in "reverse order."

I'm posting my "restatement" of Neutone's algorithm. I use these for both Rx & Tx so I can't load the buffer right there, as he does, but it's otherwise his code. I'm posting it so that others can choose between tables & bit shifting.

But here's what's weird: I can't find any time difference in using a table vs. bit shifting on my PIC18F8720 (20 Mhz). I can't imagine how this is true; has anyone else timed the two methods?

Thanks again,

Smile Robert



Code:

// Function Prototype:
int16       tablecr2 ( char * message, int8 );


/* ===========================================================================
Function Name:   tablecr2
Purpose:         utility to calculate the CRC of a buffer.  Returns an int16.

               Note: Table-based; tables are placed in program memory by CCS.

               ============================================================================*/

/* Table of CRC values for high order byte */
const char Table_CRC_Hi[256] = {
0x00, 0xC1, 0x81, 0x40, 0x01, 0xC0, 0x80, 0x41, 0x01, 0xC0, 0x80, 0x41, 0x00, 0xC1, 0x81,
0x40, 0x01, 0xC0, 0x80, 0x41, 0x00, 0xC1, 0x81, 0x40, 0x00, 0xC1, 0x81, 0x40, 0x01, 0xC0,
0x80, 0x41, 0x01, 0xC0, 0x80, 0x41, 0x00, 0xC1, 0x81, 0x40, 0x00, 0xC1, 0x81, 0x40, 0x01,
0xC0, 0x80, 0x41, 0x00, 0xC1, 0x81, 0x40, 0x01, 0xC0, 0x80, 0x41, 0x01, 0xC0, 0x80, 0x41,
0x00, 0xC1, 0x81, 0x40, 0x01, 0xC0, 0x80, 0x41, 0x00, 0xC1, 0x81, 0x40, 0x00, 0xC1, 0x81,
0x40, 0x01, 0xC0, 0x80, 0x41, 0x00, 0xC1, 0x81, 0x40, 0x01, 0xC0, 0x80, 0x41, 0x01, 0xC0,
0x80, 0x41, 0x00, 0xC1, 0x81, 0x40, 0x00, 0xC1, 0x81, 0x40, 0x01, 0xC0, 0x80, 0x41, 0x01,
0xC0, 0x80, 0x41, 0x00, 0xC1, 0x81, 0x40, 0x01, 0xC0, 0x80, 0x41, 0x00, 0xC1, 0x81, 0x40,
0x00, 0xC1, 0x81, 0x40, 0x01, 0xC0, 0x80, 0x41, 0x01, 0xC0, 0x80, 0x41, 0x00, 0xC1, 0x81,
0x40, 0x00, 0xC1, 0x81, 0x40, 0x01, 0xC0, 0x80, 0x41, 0x00, 0xC1, 0x81, 0x40, 0x01, 0xC0,
0x80, 0x41, 0x01, 0xC0, 0x80, 0x41, 0x00, 0xC1, 0x81, 0x40, 0x00, 0xC1, 0x81, 0x40, 0x01,
0xC0, 0x80, 0x41, 0x01, 0xC0, 0x80, 0x41, 0x00, 0xC1, 0x81, 0x40, 0x01, 0xC0, 0x80, 0x41,
0x00, 0xC1, 0x81, 0x40, 0x00, 0xC1, 0x81, 0x40, 0x01, 0xC0, 0x80, 0x41, 0x00, 0xC1, 0x81,
0x40, 0x01, 0xC0, 0x80, 0x41, 0x01, 0xC0, 0x80, 0x41, 0x00, 0xC1, 0x81, 0x40, 0x01, 0xC0,
0x80, 0x41, 0x00, 0xC1, 0x81, 0x40, 0x00, 0xC1, 0x81, 0x40, 0x01, 0xC0, 0x80, 0x41, 0x01,
0xC0, 0x80, 0x41, 0x00, 0xC1, 0x81, 0x40, 0x00, 0xC1, 0x81, 0x40, 0x01, 0xC0, 0x80, 0x41,
0x00, 0xC1, 0x81, 0x40, 0x01, 0xC0, 0x80, 0x41, 0x01, 0xC0, 0x80, 0x41, 0x00, 0xC1, 0x81,
0x40
} ;

/* Table of CRC values for low order byte */
const char Table_CRC_Lo[256] = {
0x00, 0xC0, 0xC1, 0x01, 0xC3, 0x03, 0x02, 0xC2, 0xC6, 0x06, 0x07, 0xC7, 0x05, 0xC5, 0xC4,
0x04, 0xCC, 0x0C, 0x0D, 0xCD, 0x0F, 0xCF, 0xCE, 0x0E, 0x0A, 0xCA, 0xCB, 0x0B, 0xC9, 0x09,
0x08, 0xC8, 0xD8, 0x18, 0x19, 0xD9, 0x1B, 0xDB, 0xDA, 0x1A, 0x1E, 0xDE, 0xDF, 0x1F, 0xDD,
0x1D, 0x1C, 0xDC, 0x14, 0xD4, 0xD5, 0x15, 0xD7, 0x17, 0x16, 0xD6, 0xD2, 0x12, 0x13, 0xD3,
0x11, 0xD1, 0xD0, 0x10, 0xF0, 0x30, 0x31, 0xF1, 0x33, 0xF3, 0xF2, 0x32, 0x36, 0xF6, 0xF7,
0x37, 0xF5, 0x35, 0x34, 0xF4, 0x3C, 0xFC, 0xFD, 0x3D, 0xFF, 0x3F, 0x3E, 0xFE, 0xFA, 0x3A,
0x3B, 0xFB, 0x39, 0xF9, 0xF8, 0x38, 0x28, 0xE8, 0xE9, 0x29, 0xEB, 0x2B, 0x2A, 0xEA, 0xEE,
0x2E, 0x2F, 0xEF, 0x2D, 0xED, 0xEC, 0x2C, 0xE4, 0x24, 0x25, 0xE5, 0x27, 0xE7, 0xE6, 0x26,
0x22, 0xE2, 0xE3, 0x23, 0xE1, 0x21, 0x20, 0xE0, 0xA0, 0x60, 0x61, 0xA1, 0x63, 0xA3, 0xA2,
0x62, 0x66, 0xA6, 0xA7, 0x67, 0xA5, 0x65, 0x64, 0xA4, 0x6C, 0xAC, 0xAD, 0x6D, 0xAF, 0x6F,
0x6E, 0xAE, 0xAA, 0x6A, 0x6B, 0xAB, 0x69, 0xA9, 0xA8, 0x68, 0x78, 0xB8, 0xB9, 0x79, 0xBB,
0x7B, 0x7A, 0xBA, 0xBE, 0x7E, 0x7F, 0xBF, 0x7D, 0xBD, 0xBC, 0x7C, 0xB4, 0x74, 0x75, 0xB5,
0x77, 0xB7, 0xB6, 0x76, 0x72, 0xB2, 0xB3, 0x73, 0xB1, 0x71, 0x70, 0xB0, 0x50, 0x90, 0x91,
0x51, 0x93, 0x53, 0x52, 0x92, 0x96, 0x56, 0x57, 0x97, 0x55, 0x95, 0x94, 0x54, 0x9C, 0x5C,
0x5D, 0x9D, 0x5F, 0x9F, 0x9E, 0x5E, 0x5A, 0x9A, 0x9B, 0x5B, 0x99, 0x59, 0x58, 0x98, 0x88,
0x48, 0x49, 0x89, 0x4B, 0x8B, 0x8A, 0x4A, 0x4E, 0x8E, 0x8F, 0x4F, 0x8D, 0x4D, 0x4C, 0x8C,
0x44, 0x84, 0x85, 0x45, 0x87, 0x47, 0x46, 0x86, 0x82, 0x42, 0x43, 0x83, 0x41, 0x81, 0x80,
0x40
};

// =============================================================================
int16 tablecr2 ( char * message, int8 length )
{
   // Global Data Declarations:
   // I'll be doing this so often that the temps might as well be global: (faster)
   static char    CRC_Lo,
                  CRC_Hi;               // "static" means "global" in CCS-land
   static int8      CRC_tableindex;
   static int16   CRC_return;
   static int8    CRC_msgindex;

   // Local Data Definitions:

   // Begin Procedure ==========================================================

   CRC_Hi             = 0xFF;
   CRC_Lo             = 0xFF;
   CRC_msgindex       = 0;

   while( CRC_msgindex < length )                    // e.g., length==6 means 0..5
   {
        CRC_tableindex = message[CRC_msgindex++];
      CRC_tableindex ^= CRC_Lo;
      CRC_Lo          = Table_CRC_Hi[CRC_tableindex];
      CRC_Lo          ^= CRC_Hi;
      CRC_Hi          = Table_CRC_Lo[CRC_tableindex];
   }

   CRC_return         = CRC_Hi;
   CRC_return          = (CRC_return<<8) | CRC_Lo;
   return             CRC_return;

}    // end <tablecr2>
// =============================================================================


Neutone



Joined: 08 Sep 2003
Posts: 839
Location: Houston

View user's profile Send private message

PostPosted: Wed Apr 07, 2004 1:45 pm     Reply with quote

To solve for timing I turned on a pin before performing the CRC and off right after. I took the pulse time and divided by the known packet size in bytes. I never did this for the lookup table method but looking at the assembly it looked like around 20 to 40 instruction per byte. If you are seeing no change in timing then you might be measuring the wrong thing.
RKnapp



Joined: 23 Feb 2004
Posts: 51

View user's profile Send private message

PostPosted: Wed Apr 07, 2004 1:57 pm     Reply with quote

I have to admit that your method of measuring time is much better; I'm not directly measuring calculation times as you did. I was measuring "duty cycle" of the processor, which includes many other things as well as this calculation.

Because I'm implementing a real-time loop, I strive for 30% max usage in the loop, with 70% "idling time." My last system was marginal, with 50% duty cycle -- some frame overruns were noted.

Thank you very much,

Smile Robert
Neutone



Joined: 08 Sep 2003
Posts: 839
Location: Houston

View user's profile Send private message

PostPosted: Wed Apr 14, 2004 3:59 pm     Reply with quote

I did some further measurements to CRC calculation time. Basic bit shifting was running around 150 instruction(per byte). Using a ROM base lookup table it takes around 75 instructions. With a RAM based lookup table it takes around 35 instructions. This is all done testing the on time of a pin during a 100 byte packet being encoded.
Display posts from previous:   
Post new topic   Reply to topic    CCS Forum Index -> General CCS C Discussion All times are GMT - 6 Hours
Page 1 of 1

 
Jump to:  
You cannot post new topics in this forum
You cannot reply to topics in this forum
You cannot edit your posts in this forum
You cannot delete your posts in this forum
You cannot vote in polls in this forum


Powered by phpBB © 2001, 2005 phpBB Group