Page 1 of 1

hidtool works on Linux, does not work on Windows 7

Posted: Wed Dec 18, 2013 12:57 pm
by filo
Hello,
I made a device based on the hid-data demo. It works great on Linux (tested on two machines), but hidtool read fails on Windows 7 64-bit (also tested on two different machines other then the Linux ones):

Code: Select all

error reading data: Communication error with device

When I unplug it hidtool returns device not found.
The device is correctly enumerated and shown in device manager without warnings.
Hardware itself is not faulty, as I successfully used bootloadHID to upload new firmware in Windows 7.

The main.c:

Code: Select all

/* Name: main.c
 * Project: hid-data, example how to use HID for data transfer
 * Author: Christian Starkjohann
 * Creation Date: 2008-04-11
 * Tabsize: 4
 * Copyright: (c) 2008 by OBJECTIVE DEVELOPMENT Software GmbH
 * License: GNU GPL v2 (see License.txt), GNU GPL v3 or proprietary (CommercialLicense.txt)
 */

//demo modified by SP2AGX for yaesu antenna switch

/*
This example should run on most AVRs with only little changes. No special
hardware resources except INT0 are used. You may have to change usbconfig.h for
different I/O pins for USB. Please note that USB D+ must be the INT0 pin, or
at least be connected to INT0 as well.
*/

#include <stdlib.h>
#include <avr/io.h>
#include <avr/wdt.h>
#include <avr/interrupt.h>  /* for sei() */
#include <util/delay.h>     /* for _delay_ms() */
#include <avr/eeprom.h>
#include <avr/pgmspace.h>   /* required by usbdrv.h */
#include "usbdrv.h"

/* ----------- application specific defines --------- */
#define LED_PTT_ON()   PORTD&=~_BV(PD4)
#define LED_PTT_OFF()  PORTD|=_BV(PD4)
#define LED_MODE_ON()  PORTB&=~_BV(PB5)
#define LED_MODE_OFF() PORTB|=_BV(PB5)
#define NO_PTT PINC&_BV(PC1)
#define MODE_BUTTON_UP PINB&_BV(PB4)
#define MODE_BUTTON_DOWN !(PINB&_BV(PB4))
#define MODE_AUTO 1
#define MODE_MANUAL 0
/* ----------- runtime variables -------------------- */
//number of samples to average
#define ADC_SAMPLES_MAX 16
//there are 7 buttons, button 0 means none pressed (8 possible input states)
#define BUTTONS_N 8
unsigned char volatile adc_values[ADC_SAMPLES_MAX];
unsigned char volatile adc_index=0;
unsigned char volatile process_keys=0;
unsigned char volatile demanded_antenna=1;
unsigned char volatile selected_antenna=0;
unsigned char volatile operation_mode=MODE_AUTO;
/* ----------- EEPROM variables --------------------- */
#define BAND_COUNT 17
unsigned char ee_band_antenna[BAND_COUNT] EEMEM;

//done: ports initialization
//done: driver control
//done: radio interfacing + PTT lock
//done: key sampling
//done: memory reset
//done: EEPROM programming
//done: USB routines

//prototypes
void switch_antenna(unsigned char n);
inline unsigned char get_band_input();
unsigned char get_button_pressed();

/* ------------------------------------------------------------------------- */
/* ----------------------------- USB interface ----------------------------- */
/* ------------------------------------------------------------------------- */

PROGMEM const char usbHidReportDescriptor[22] = {    /* USB report descriptor */
    0x06, 0x00, 0xff,              // USAGE_PAGE (Generic Desktop)
    0x09, 0x01,                    // USAGE (Vendor Usage 1)
    0xa1, 0x01,                    // COLLECTION (Application)
    0x15, 0x00,                    //   LOGICAL_MINIMUM (0)
    0x26, 0xff, 0x00,              //   LOGICAL_MAXIMUM (255)
    0x75, 0x08,                    //   REPORT_SIZE (8)
    0x95, 0x10,                    //   REPORT_COUNT (16) changed!!!!!!!
    0x09, 0x00,                    //   USAGE (Undefined)
    0xb2, 0x02, 0x01,              //   FEATURE (Data,Var,Abs,Buf)
    0xc0                           // END_COLLECTION
};
/* Since we define only one feature report, we don't use report-IDs (which
 * would be the first byte of the report). The entire report consists of 128
 * opaque data bytes.
 */

/* The following variables store the status of the current data transfer */
//static uchar    currentAddress;
//static uchar    bytesRemaining;
//static uchar    page_flag=0;

/* ------------------------------------------------------------------------- */


/* usbFunctionRead() is called when the host requests a chunk of data from
 * the device. For more information see the documentation in usbdrv/usbdrv.h.
 */
uchar   usbFunctionRead(uchar *data, uchar len)
{

  data[0]=operation_mode;
  data[1]=selected_antenna;
  data[2]=demanded_antenna;
  data[3]=get_band_input();
  data[4]=get_button_pressed();
  data[5]=ADCH;
  data[6]=NO_PTT;
  data[7]=0;
  return 8;
}

/* usbFunctionWrite() is called when the host sends a chunk of data to the
 * device. For more information see the documentation in usbdrv/usbdrv.h.
 */
uchar   usbFunctionWrite(uchar *data, uchar len)
{
  if (data[0]=='M'){ // 0x4D
    if (data[1]==1){ //switch to manual mode
      operation_mode=MODE_MANUAL;
      demanded_antenna=data[2];
      switch_antenna(demanded_antenna);
    }
  } else if (data[0]=='A'){// 0x41
    operation_mode=MODE_AUTO;//return to auto mode
  }
    return 1;
}

/* ------------------------------------------------------------------------- */

usbMsgLen_t usbFunctionSetup(uchar data[8])
{
usbRequest_t    *rq = (void *)data;

    if((rq->bmRequestType & USBRQ_TYPE_MASK) == USBRQ_TYPE_CLASS){    /* HID class request */
        if(rq->bRequest == USBRQ_HID_GET_REPORT){  /* wValue: ReportType (highbyte), ReportID (lowbyte) */
            /* since we have only one report type, we can ignore the report-ID */
       //page_flag=0;
            return USB_NO_MSG;  /* use usbFunctionRead() to obtain data */
        }else if(rq->bRequest == USBRQ_HID_SET_REPORT){
       //page_flag=0;
            /* since we have only one report type, we can ignore the report-ID */
            return USB_NO_MSG;  /* use usbFunctionWrite() to receive data from host */
        }
    }else{
        /* ignore vendor type requests, we don't use any */
    }
    return 0;
}

/* ------------------------------------------------------------------------- */
//switches the selected antenna relay if not blocked by PTT
void switch_antenna(unsigned char n){
  if (NO_PTT){ // PTT not active
    if (selected_antenna!=n){ //if there is any need to change
      PORTD&=~(_BV(PD5)|_BV(PD6)|_BV(PD7));          //all antennas
      PORTB&=~(_BV(PB0)|_BV(PB1)|_BV(PB2)|_BV(PB3)); //    off
      switch(n){
        case 1:PORTB|=_BV(PB1);break; //PB1
        case 2:PORTB|=_BV(PB2);break; //PB2
        case 3:PORTB|=_BV(PB3);break; //PB3
        case 4:PORTB|=_BV(PB0);break; //PB0
        case 5:PORTD|=_BV(PD7);break; //PD7
        case 6:PORTD|=_BV(PD6);break; //PD6
        case 7:PORTD|=_BV(PD5);break; //PD5
      }
    selected_antenna=n;
    }
  }
}
/* ------------------------------------------------------------------------- */
//returns the selected band from transceiver output
inline unsigned char get_band_input(){
  return (PINC>>2)&0xF; //read the inputs, shift to lower the value and mask high bits
}

/* ------------------------------------------------------------------------- */
//averages ADC samples and picks the most likely pressed button
unsigned char get_button_pressed(){
 uint16_t error_table[BUTTONS_N];
 unsigned char max_adc=0,min_adc=255;
 unsigned char i;
 unsigned char adc_value_avg;
 static const unsigned char perfect_value[]={0, 0xFF, 0xDB, 0xB6, 0x91, 0x6D, 0x48, 0x24 };//zero is simply lack of keys, constant pulldown resistor
 unsigned min_err=0xFFFF,min_err_i=0; //by default assume no key pressed
 
  for (i=0;i<ADC_SAMPLES_MAX;i++){
    if (adc_values[i]>max_adc){ max_adc=adc_values[i]; }
    if (adc_values[i]<min_adc){ min_adc=adc_values[i]; }
  }
 
  if (max_adc-min_adc < 10 ){ //all ADC values look stable, 10 is just a magic number
    adc_value_avg=((uint16_t)max_adc+min_adc)/2;
 
    for (i=0;i<BUTTONS_N;i++){ //compute differences between ADC and perfect key values for each key
      if (adc_value_avg>perfect_value[i]){ //prevents underflow at zero
   error_table[i] =  adc_value_avg - perfect_value[i];
      } else {
   error_table[i] =  perfect_value[i] - adc_value_avg;
      }
    }
   
    for (i=0;i<BUTTONS_N;i++){ //pick the smallest error
      if (error_table[i]<min_err) {
   min_err=error_table[i];
   min_err_i=i;
      }
    }
  }
  return min_err_i;
}
/* ------------------------------------------------------------------------- */
//reset to defaults, then halt
void memory_reset(){
   unsigned char i=0;
   for (i=0;i<BAND_COUNT ;i++){
     eeprom_busy_wait();
     eeprom_write_byte(&ee_band_antenna[i],1);
   }
   LED_MODE_ON();
   LED_PTT_ON();
   PORTD|=_BV(PD6);//some extra leds
   PORTD|=_BV(PD5);
   while (1){
      cli();
      wdt_reset(); //endless loop, wait for power cycle
   }
 
}
/* ------------------------------------------------------------------------- */
int main(void)
{

  wdt_reset();
  wdt_enable(WDTO_1S);

    /* Even if you don't use the watchdog, turn it off here. On newer devices,
     * the status of the watchdog (on/off, period) is PRESERVED OVER RESET!
     */
    /* RESET status: all port bits are inputs without pull-up.
     * That's the way we need D+ and D-. Therefore we don't need any
     * additional hardware initialization.
     */

    usbInit();
    usbDeviceDisconnect();  /* enforce re-enumeration, do this while interrupts are disabled! */

    /* ----- ADC configuration ----- */
    ADMUX = _BV(REFS0)|_BV(ADLAR);//multiplexed button input ADC0, Aref=AVCC+cap, 8-bit left adjust
    ADCSRA = _BV(ADEN) | _BV(ADPS2) |  _BV(ADPS1) | _BV(ADPS0) | _BV(ADSC); //93kHz ADC clock, interrupt enable, start conversion

    /* ----- pins configuration ----- */
    PORTC|=_BV(PC1)|_BV(PC2)|_BV(PC3)|_BV(PC4)|_BV(PC5); //PTT and band input pullup
    DDRD|= _BV(PD4)|_BV(PD5)|_BV(PD6)|_BV(PD7); //PTT_LED and relay driver output
    DDRB|= _BV(PB0)|_BV(PB1)|_BV(PB2)|_BV(PB3)|_BV(PB5); //MODE_LED and relay driver output
    LED_MODE_OFF();
    LED_PTT_OFF();
    PORTB|=_BV(PB4); //mode button input pullup

    /* ---timer configuration for key sampling --- */
    TCCR0 = _BV(CS02); // 256 prescaling, ~180Hz interrupt rate
    TIMSK = _BV(TOIE0);
   
    loop_until_bit_is_set(ADCSRA,ADIF);//wait for the first ADC conversion
    if (ADCH>200){
      memory_reset();
    }
    ADCSRA |= _BV(ADIE); //enable ADC interrupt
   
    unsigned char i = 0;
    while(--i){             /* fake USB disconnect for > 250 ms */
        wdt_reset();
        _delay_ms(1);
    }
    usbDeviceConnect();
    sei();
   
    for(;;){                /* main event loop */
        wdt_reset();
        usbPoll();

   if (process_keys){ //slow processing loop, ~180Hz rate
     process_keys=0;
     unsigned char button=get_button_pressed();
    
     if (button){ //any button pressed
       if (MODE_BUTTON_DOWN){ //memory programming
         eeprom_busy_wait();
         eeprom_write_byte(&ee_band_antenna[get_band_input()],button);
         eeprom_busy_wait();
         operation_mode=MODE_AUTO;
       } else {      //regular manual switch
         operation_mode=MODE_MANUAL;
         demanded_antenna=button;
       }
     }
    
     if (operation_mode==MODE_AUTO){
       demanded_antenna=eeprom_read_byte(&ee_band_antenna[get_band_input()]); //retrive the programmed antenna from eeprom
       LED_MODE_ON();
     } else {
       LED_MODE_OFF();
     }
    
     if (MODE_BUTTON_DOWN){
       operation_mode=MODE_AUTO;
     }
   }

   if (NO_PTT){
     LED_PTT_OFF();
   } else {
     LED_PTT_ON();
   }
   
   switch_antenna(demanded_antenna);
    }
    return 0;
}

/* ------------------------------------------------------------------------- */
//key sampling interrupt

ISR (TIMER0_OVF_vect, ISR_NOBLOCK){ //interrupt should be interruptable
  TIMSK &= ~_BV(TOIE0);//disable timer interrupt
  ADCSRA |= _BV(ADSC); //start next conversion
}

ISR (ADC_vect, ISR_NOBLOCK){ //interrupt should be interruptable
        adc_values[adc_index]=ADCH; //store current value
   adc_index++;
   if (adc_index==ADC_SAMPLES_MAX){
     adc_index=0;
     process_keys=1;
   }
   TCNT0 = 0; //reset delay timer
   TIMSK |= _BV(TOIE0);//enable timer interrupt
}


I also tried changing REPORT_COUNT (16) back to defaults, though I've built a different device using an identical descriptor that actually works under Windows.

I did a communication dump using Wireshark:
http://sp2put.utp.edu.pl/~filo/yas/yas_usbdump.pcapng
The device is probably connected after packet 53.
I try to execute hidtool read at packet 77 and then again at packet 83.

The device itself is a little more described at http://sp2put.utp.edu.pl/index.php/2761,automatyczny-przelacznik-anten-do-yaesu-ft-950-z-usb?lang=en

Re: hidtool works on Linux, does not work on Windows 7

Posted: Wed Dec 18, 2013 3:55 pm
by filo
Everything also works perfectly under OS X Maverics.

Re: hidtool works on Linux, does not work on Windows 7

Posted: Thu Dec 19, 2013 7:53 pm
by filo
I figured out that the only change in hidtool.c I made is at line 94 - I decreased buffer size to print just 8 bytes (they wrap around at the device anyway). I reverted back to 129 bytes as in the stock demo and now it works on Windows. I still don't get why it worked modified on Unixes.

Re: hidtool works on Linux, does not work on Windows 7

Posted: Tue Dec 24, 2013 10:04 pm
by xiangrui
I also started with this tool. It seems to me that Windows system is more strict on following the descriptor. The data sent and received must have the length defined in the descriptor, otherwise it will report error. Under unix, you can read and write less data than defined. You may get a warning at some level of the driver.