Skip to content

Commit b34fff2

Browse files
committed
Fixing HID-IO controller unit tests
- Fixed continued packets to send all at the same time (while still chunking into max packet size) - Updated tests to reflect this - Tests now passing
1 parent c42d1e9 commit b34fff2

File tree

3 files changed

+116
-203
lines changed

3 files changed

+116
-203
lines changed

Diff for: Output/HID-IO/hidio_com.c

+78-49
Original file line numberDiff line numberDiff line change
@@ -144,29 +144,41 @@ extern var_uint_t macroTriggerEventLayerCache[];
144144
// If there is a wrap-around, the buffer is copied.
145145
// buf_pos - Index position in the buffer
146146
// len - Length of the desired buffer
147+
typedef struct HIDIO_Munch {
148+
uint8_t *buf; // Pointer to memory of packet
149+
uint16_t next_pos; // Position of next packet
150+
} HIDIO_Munch;
147151
//
148152
// A pointer the data is returned, which may or may not be the provided buffer
149-
uint8_t *HIDIO_buffer_munch( HIDIO_Buffer *buffer, uint8_t *buf, uint16_t buf_pos, uint16_t len )
153+
HIDIO_Munch HIDIO_buffer_munch( HIDIO_Buffer *buffer, uint8_t *buf, uint16_t buf_pos, uint16_t len )
150154
{
155+
HIDIO_Munch munch;
156+
151157
// Determine if buffer is contiguous for the length
152158
if ( buf_pos + len < buffer->len )
153159
{
154160
// We can just set the buffer directly
155-
return &(buffer->data[ buf_pos ]);
161+
munch.buf = &(buffer->data[ buf_pos ]);
162+
munch.next_pos = buf_pos + len;
163+
return munch;
156164
}
157165

158166
// If just a single byte, just return the first position (wrap-around)
159167
if ( len == 1 )
160168
{
161-
return &(buffer->data[0]);
169+
munch.buf = &(buffer->data[0]);
170+
munch.next_pos = buf_pos + 1;
171+
return munch;
162172
}
163173

164174
// Copy into the buffer
165175
uint16_t cur_len = buffer->len - buf_pos;
166176
memcpy( buf, &(buffer->data[ buf_pos ]), cur_len );
167177
memcpy( &buf[ cur_len ], buffer->data, len - cur_len );
168178

169-
return buf;
179+
munch.buf = buf;
180+
munch.next_pos = len - cur_len;
181+
return munch;
170182
}
171183

172184
// Push byte to ring buffer
@@ -334,28 +346,30 @@ uint8_t HIDIO_id_width( uint32_t id )
334346

335347
// Generate packet
336348
// This function can be called multiple times to generate a full packet
337-
// Continue to call unitil the value returned equals payload_len
349+
// Continue to call until the value returned equals payload_len
338350
// If 0 is returned, there has been an error, and the packet is aborted.
339351
// To start a new packet, start at pos = 0
340352
uint16_t HIDIO_buffer_generate_packet(
341-
HIDIO_Buffer *buf,
342-
uint16_t pos,
343-
uint16_t payload_len,
344-
uint8_t *data,
345-
uint16_t data_len,
346-
HIDIO_Packet_Type type,
347-
uint32_t id
353+
HIDIO_Buffer *buf, // Buffer to use
354+
uint16_t pos, // Position in the packet
355+
uint16_t payload_len, // Total length of payload (if larger than packet size, will be a continued packet)
356+
uint8_t *data, // Pointer to data being copied into buffer
357+
uint16_t data_len, // Length of buffer being copied in
358+
HIDIO_Packet_Type type, // Type of packet
359+
uint32_t id // Packet id
348360
)
349361
{
350-
/*print("head: ");
362+
/*
363+
print("head: ");
351364
printInt16( buf->head );
352365
print(" tail: ");
353366
printInt16( buf->tail );
354367
print(" bytes_left: ");
355368
printInt16( HIDIO_buffer_free_bytes( buf ) );
356369
print(" request: ");
357370
printInt16( data_len );
358-
print(NL);*/
371+
print(NL);
372+
*/
359373

360374
// Determine payload max
361375
uint8_t width = HIDIO_id_width( id );
@@ -651,7 +665,7 @@ HIDIO_Return HIDIO_test_2_call( uint16_t buf_pos, uint8_t irq )
651665

652666
// Munch buffer entry header, not including data
653667
uint8_t tmpbuf[ sizeof(HIDIO_Buffer_Entry) ];
654-
uint8_t *buf = HIDIO_buffer_munch( &HIDIO_assembly_buf, (uint8_t*)&tmpbuf, buf_pos, sizeof(HIDIO_Buffer_Entry) );
668+
uint8_t *buf = HIDIO_buffer_munch( &HIDIO_assembly_buf, (uint8_t*)&tmpbuf, buf_pos, sizeof(HIDIO_Buffer_Entry) ).buf;
655669
HIDIO_Buffer_Entry *entry = (HIDIO_Buffer_Entry*)buf;
656670

657671
// Make sure entry is ready
@@ -667,7 +681,7 @@ HIDIO_Return HIDIO_test_2_call( uint16_t buf_pos, uint8_t irq )
667681
{
668682
uint8_t buf;
669683
uint16_t calc_buf_pos = HIDIO_buffer_position( &HIDIO_assembly_buf, buf_pos + sizeof(HIDIO_Buffer_Entry), pos );
670-
uint8_t *byte = HIDIO_buffer_munch( &HIDIO_assembly_buf, &buf, calc_buf_pos, 1 );
684+
uint8_t *byte = HIDIO_buffer_munch( &HIDIO_assembly_buf, &buf, calc_buf_pos, 1 ).buf;
671685

672686
// Count transitions, should only be 1, from 0 to value
673687
if ( *byte != last_byte )
@@ -759,7 +773,7 @@ HIDIO_Return HIDIO_terminal_call( uint16_t buf_pos, uint8_t irq )
759773

760774
// Munch buffer entry header, not including data
761775
uint8_t tmpbuf[ sizeof(HIDIO_Buffer_Entry) ];
762-
uint8_t *buf = HIDIO_buffer_munch( &HIDIO_assembly_buf, (uint8_t*)&tmpbuf, buf_pos, sizeof(HIDIO_Buffer_Entry) );
776+
uint8_t *buf = HIDIO_buffer_munch( &HIDIO_assembly_buf, (uint8_t*)&tmpbuf, buf_pos, sizeof(HIDIO_Buffer_Entry) ).buf;
763777
HIDIO_Buffer_Entry *entry = (HIDIO_Buffer_Entry*)buf;
764778

765779
// Make sure entry is ready
@@ -786,7 +800,7 @@ HIDIO_Return HIDIO_terminal_call( uint16_t buf_pos, uint8_t irq )
786800

787801
uint8_t buf;
788802
uint16_t calc_buf_pos = HIDIO_buffer_position( &HIDIO_assembly_buf, buf_pos + sizeof(HIDIO_Buffer_Entry), pos );
789-
uint8_t *byte = HIDIO_buffer_munch( &HIDIO_assembly_buf, &buf, calc_buf_pos, 1 );
803+
uint8_t *byte = HIDIO_buffer_munch( &HIDIO_assembly_buf, &buf, calc_buf_pos, 1 ).buf;
790804
CLILineBuffer[CLILineBufferCurrent++] = *byte;
791805
}
792806

@@ -895,7 +909,7 @@ HIDIO_Return HIDIO_call_id( uint32_t id, uint16_t buf_pos, uint8_t irq )
895909
{
896910
// HIDIO_Return__Ok, we can pop the most recent assembly_buf packet
897911
case HIDIO_Return__Ok:
898-
entry = (HIDIO_Buffer_Entry*)HIDIO_buffer_munch( &HIDIO_assembly_buf, tmpdata, HIDIO_assembly_buf.head, sizeof(tmpdata) );
912+
entry = (HIDIO_Buffer_Entry*)HIDIO_buffer_munch( &HIDIO_assembly_buf, tmpdata, HIDIO_assembly_buf.head, sizeof(tmpdata) ).buf;
899913

900914
// Determine size of data
901915
datasize = sizeof(HIDIO_Buffer_Entry) + entry->size;
@@ -958,29 +972,44 @@ HIDIO_Return HIDIO_reply_id( uint32_t id, uint8_t *buf, uint8_t irq )
958972
// Enough space to store header
959973
uint8_t tmpdata[6];
960974
uint16_t datasize;
975+
uint8_t continued;
961976
HIDIO_Packet *packet;
962977

963978
switch ( retval )
964979
{
965980
// HIDIO_Return__Ok, we can pop the most recent tx_buf packet
966981
case HIDIO_Return__Ok:
967-
packet = (HIDIO_Packet*)HIDIO_buffer_munch( &HIDIO_tx_buf, tmpdata, HIDIO_tx_buf.head, sizeof(tmpdata) );
982+
// Cleanup until a non-continued packet
983+
while ( HIDIO_tx_buf.packets_ready > 0 )
984+
{
985+
packet = (HIDIO_Packet*)HIDIO_buffer_munch( &HIDIO_tx_buf, tmpdata, HIDIO_tx_buf.head, sizeof(tmpdata) ).buf;
968986

969-
// Determine size of data
970-
datasize = (packet->upper_len << 8) | packet->len;
987+
// Determine size of data
988+
datasize = (packet->upper_len << 8) | packet->len;
971989

972-
// Pop bytes and decrement packet ready counter
973-
if ( HIDIO_buffer_pop_bytes( &HIDIO_tx_buf, datasize + 2 ) )
974-
{
975-
HIDIO_tx_buf.packets_ready--;
976-
}
977-
// Failed pop, generally popping more buffer than is available
978-
// (this is very bad, but recovering anyways)
979-
else
980-
{
981-
HIDIO_tx_buf.packets_ready = 0;
982-
HIDIO_tx_buf.head = 0;
983-
HIDIO_tx_buf.tail = 0;
990+
// Check if continued
991+
continued = packet->cont;
992+
993+
// Pop bytes and decrement packet ready counter
994+
if ( HIDIO_buffer_pop_bytes( &HIDIO_tx_buf, datasize + 2 ) )
995+
{
996+
HIDIO_tx_buf.packets_ready--;
997+
}
998+
// Failed pop, generally popping more buffer than is available
999+
// (this is very bad, but recovering anyways)
1000+
else
1001+
{
1002+
HIDIO_tx_buf.packets_ready = 0;
1003+
HIDIO_tx_buf.head = 0;
1004+
HIDIO_tx_buf.tail = 0;
1005+
break;
1006+
}
1007+
1008+
// If there aren't any more packets (continued packets only), stop
1009+
if ( !continued )
1010+
{
1011+
break;
1012+
}
9841013
}
9851014

9861015
// Unset waiting for ACK packet
@@ -1151,7 +1180,7 @@ void HIDIO_process_incoming_packet( uint8_t *buf, uint8_t irq )
11511180
// We first determine if we're reassembling in the data or ack buffers
11521181
case HIDIO_Packet_Type__Continued:
11531182
// Modify type so we know what to do with the payload
1154-
type = !HIDIO_ack_buf->done ? HIDIO_Packet_Type__Data : HIDIO_Packet_Type__ACK;
1183+
type = HIDIO_assembly_buf.waiting ? HIDIO_Packet_Type__Data : HIDIO_Packet_Type__ACK;
11551184

11561185
// Most packet types
11571186
default:
@@ -1223,7 +1252,7 @@ void HIDIO_process_incoming_packet( uint8_t *buf, uint8_t irq )
12231252
tmpbuf,
12241253
HIDIO_assembly_buf.cur_buf_head,
12251254
sizeof(tmpbuf)
1226-
);
1255+
).buf;
12271256

12281257
// Update entry
12291258
entry->size += payload_len;
@@ -1283,11 +1312,6 @@ void HIDIO_process_incoming_packet( uint8_t *buf, uint8_t irq )
12831312
break;
12841313
}
12851314
}
1286-
// Otherwise do a simple ACK
1287-
else
1288-
{
1289-
//HIDIO_nopayload_ack( id );
1290-
}
12911315
break;
12921316

12931317
case HIDIO_Packet_Type__ACK:
@@ -1308,7 +1332,6 @@ void HIDIO_process_incoming_packet( uint8_t *buf, uint8_t irq )
13081332
HIDIO_reply_id( id, (uint8_t*)HIDIO_ack_buf, irq );
13091333
}
13101334
break;
1311-
break;
13121335

13131336
default:
13141337
// TODO (HaaTa)
@@ -1355,7 +1378,7 @@ inline void HIDIO_process()
13551378
// Prepare 64 byte packet
13561379
// TODO (HaaTa): Handle internal max size
13571380
uint8_t tmpdata[64];
1358-
uint8_t *buf = HIDIO_buffer_munch( &HIDIO_ack_send_buf, tmpdata, HIDIO_ack_send_buf.head, HIDIO_Packet_Size );
1381+
uint8_t *buf = HIDIO_buffer_munch( &HIDIO_ack_send_buf, tmpdata, HIDIO_ack_send_buf.head, HIDIO_Packet_Size ).buf;
13591382
HIDIO_Packet *packet = (HIDIO_Packet*)buf;
13601383

13611384
// Send packet
@@ -1385,21 +1408,27 @@ inline void HIDIO_process()
13851408

13861409
// Send outgoing packet, we can only send one at a time
13871410
// and the next one can only be sent after an ACK is recieved
1388-
if ( HIDIO_tx_buf.packets_ready > 0 && HIDIO_tx_buf.waiting == 0 )
1411+
uint16_t next_packet_pos = HIDIO_tx_buf.head;
1412+
uint16_t packets_ready = HIDIO_tx_buf.packets_ready;
1413+
while ( packets_ready > 0 && HIDIO_tx_buf.waiting == 0 )
13891414
{
13901415
// Prepare 64 byte packet
13911416
// TODO (HaaTa): Handle internal max size
13921417
uint8_t tmpdata[64];
1393-
uint8_t *buf = HIDIO_buffer_munch( &HIDIO_tx_buf, tmpdata, HIDIO_tx_buf.head, HIDIO_Packet_Size );
1394-
HIDIO_Packet *packet = (HIDIO_Packet*)buf;
1418+
HIDIO_Munch munch = HIDIO_buffer_munch( &HIDIO_tx_buf, tmpdata, next_packet_pos, HIDIO_Packet_Size );
1419+
HIDIO_Packet *packet = (HIDIO_Packet*)munch.buf;
1420+
next_packet_pos = munch.next_pos;
13951421

13961422
// Send packet
13971423
// TODO (HaaTa): Check error?
13981424
Output_rawio_sendbuffer( (char*)packet );
13991425

14001426
// Indicate waiting for ACK packet
14011427
// Once ACK has been received (or NAK) the packet will be released
1402-
HIDIO_tx_buf.waiting = 1;
1428+
HIDIO_tx_buf.waiting = packet->cont ? 0 : 1;
1429+
1430+
// Next packet
1431+
packets_ready--;
14031432
}
14041433

14051434
// End latency measurement
@@ -1517,9 +1546,9 @@ void HIDIO_Open_url_capability( TriggerMacro *trigger, uint8_t state, uint8_t st
15171546
}
15181547

15191548
uint16_t payload_len;
1520-
//uint8_t arg = *(uint8_t*)(&args[0]);
15211549

15221550
#ifdef url_strings
1551+
uint8_t arg = *(uint8_t*)(&args[0]);
15231552
char* text_buf = url_strings[arg];
15241553
#else
15251554
char* text_buf = NULL;
@@ -1559,9 +1588,9 @@ void HIDIO_layout_capability( TriggerMacro *trigger, uint8_t state, uint8_t stat
15591588
}
15601589

15611590
uint16_t payload_len;
1562-
//uint8_t arg = *(uint8_t*)(&args[0]);
15631591

15641592
#ifdef layout_strings
1593+
uint8_t arg = *(uint8_t*)(&args[0]);
15651594
char* text_buf = layout_strings[arg];
15661595
#else
15671596
char* text_buf = NULL;

Diff for: Output/TestOut/host.py

+4-4
Original file line numberDiff line numberDiff line change
@@ -52,11 +52,11 @@ class HIDIO_Packet( Structure ):
5252
See hidio_com.h in Output/HID-IO
5353
'''
5454
_fields_ = [
55-
( 'type', c_uint8, 3 ),
56-
( 'cont', c_uint8, 1 ),
57-
( 'id_width', c_uint8, 1 ),
58-
( 'reserved', c_uint8, 1 ),
5955
( 'upper_len', c_uint8, 2 ),
56+
( 'reserved', c_uint8, 1 ),
57+
( 'id_width', c_uint8, 1 ),
58+
( 'cont', c_uint8, 1 ),
59+
( 'type', c_uint8, 3 ),
6060
( 'len', c_uint8 ),
6161
]
6262

0 commit comments

Comments
 (0)