Snelle threads

[carbidsjitter]

Hallo mede ontwikkelaars,

Ik ben bezig met een usb device die de canbus kan aansturen. De usb hardware meld zich aan als een Hid device, zodat er geen driver geïnstalleerd hoeft te worden.

Nu ben ik bezig met een host programma. Het zenden van data naar het device gaat goed.

Het ontvangen gaat wat moeizamer. Ik laat het device continue data verzenden naar de host.

Een thread zorgt ervoor dat de data in een array komt. Als ik deze array bekijk dan mis ik zo nu en dan data. Ik weet van de usb dat er elke 1 ms data uitgewisseld wordt.

Mijn vraag is:

Hoe krijg ik mijn thread, die de ontvangende data moet verwerken, zo dat hij gegarandeerd elke 1 ms uitgevoerd wordt ?

Ik heb al wat zitten spelen met de thread priority, maar dat geeft bij de verschillende priority's weinig verschil.

De tools die ik gebruik:
Windows 7 64 bit
Visual Studio 2005
unmanaged c++ (win32)

[Woy]

Hoe lees je de data uit? HID maakt gewoon gebruik van een (virtual) file handle, waar je gewoon uit kunt lezen. Als je af en toe data mist, dan verwacht ik eerder dat er ergens in jouw uitlees gedeelte wat mis gaat.

carbidsjitter schreef op maandag 06 december 2010 @ 19:55:
Hoe krijg ik mijn thread, die de ontvangende data moet verwerken, zo dat hij gegarandeerd elke 1 ms uitgevoerd wordt ?

Hieruit maak ik op dat je een thread hebt die de data uitleest ( Of is dat asynchroon? ), en een 2e thread die de data verwerkt? Je moet dan niet proberen om die 2e thread elke ms "uit te voeren" maar je zult iets van synchronisatie tussen de threads toe moeten voegen.

[ Voor 54% gewijzigd door Woy op 06-12-2010 20:08 ]

[voodooless]

Voor zover ik weet heeft de HID laag gewoon wat buffering. Het lijkt me dus sterk dat je data zal missen. Hier staat wel iets over buffered en unbuffered.

[Haan]

Disclaimer: ik ben niet heel erg thuis in het USB gebeuren, maar toch even mijn gedachte:

Als er iedere 1ms data verzonden wordt, zou dat betekenen dat de USB poort op 1000Hz staat ingesteld. Nu is dat als het goed is wel mogelijk, maar niet bepaald standaard. Voor zover ik weet is dat 125Hz, dus elke 8 ms nieuwe data. Zou dat er mee te maken kunnen hebben?

[MLM]

Je kan gewoon een thread starten die enkel een loopje om ReadFile doet? Dat is gewoon een blocking call, die pas returned als je data hebt ontvangen. Als je dan de data alleen in een array/collectie/whatever gooit, is die thread echt wel binnen 1ms klaar om de volgende iteratie te starten

Als je dat al doet (lijkt wel wat op jouw beschrijving), dan denk ik eerder dat je USB device niet goed de data aanlevert aan windows

[carbidsjitter]

voodooless schreef op maandag 06 december 2010 @ 20:08:
Voor zover ik weet heeft de HID laag gewoon wat buffering. Het lijkt me dus sterk dat je data zal missen. Hier staat wel iets over buffered en unbuffered.

Die buffering probeerde ik in mijn eigen code te doen, maar het leek me al sterk dat ik dit allemaal zelf moet doen.

Ik lees de usb uit met:

C++:

Result = HidD_GetInputReport(ReadHandle, InputReport,Capabilities.InputReportByteLength);

Deze functie staat in de hidsdi.h van de Driver Development Kit van windows.

Deze functie staat in een aparte thread in een while lus. Hierbij mis ik zo nu en dan data. Dit wordt erger als de thread ook andere dingen moet doen, dus ik weet bijna (ik weet het, aanname's zijn dodelijk) wel zeker dat het device goed verzend.

Maar ik heb nu het boek van jan Axelson, USB complete. Hierin staat uitgebreid in hoe je readfile kunt gebruiken, ook kun je hiervan de buffer size veranderen.

Ik heb nu in een while lus staan (niet in een apart thread, sneller kan dus niet)

C:

Result = ReadFile(ReadHandle, InputReport, 
Capabilities.InputReportByteLength, 
&NumberOfBytesRead,     
(LPOVERLAPPED) &HIDOverlapped); 

Als ik nu de array weer uitlees waar de data ingezet wordt dan lees ik bijna 1000 keer dezelfde waardes (binnen elke ms), blijkbaar is dit zeker wel snel genoeg. Ik zal hier nog wat mee proberen.

Het blijft voor mij wel een raadsel dat readfile geen data mist en met HidD_GetInputReport wel data mis. Beide op dezelfde manier gebruikt.

Ik moet wel zeggen dat ik van microcontroller wereld kom, dus ik denk nu nog vooral in C en interrupts en geen OS i.p.v C++, threads, OS en multicore's.

[Woy]

Ik heb geen ervaring met het gebruik van HidD_GetInputReport, maar kan het niet zijn dat je ReportBufferLength te klein is, en dat sommige reports groter zijn dan andere, waardoor je sommige reports mist? Bij je ReadFile oplossing zal je dan gewoon een incomplete report lezen, terwijl de GetInputReport hem misschien gewoon discard?

[ Voor 24% gewijzigd door Woy op 07-12-2010 11:00 ]

[carbidsjitter]

@woy ik heb het niet meer geprobeerd met HidD_GetInputReport maar wel met readfile.

Ik heb dit eerst nog geprobeerd om dit in een thread te doen, maar dat ging niet helemaal lekker. De thread hield gewoon op met werken. Als iemand weet hoe dit kan dan hoor ik dat graag.

Ik heb uiteindelijk gedacht, waarom een thread gebruiken als de OS het toch al buffert, en heb dus uiteindelijk geen thread gebruikt.

Voor degene die dit willen weten. Dit is uiteindelijk de ontvangst functie geworden. Ik heb het grotendeels uit het usb boek gehaald wat ik gekocht had. Dit voorbeeld werkte met een event systeem. Wanneer er niks in de buffer staat dan ontstaat er een time out en wordt een 0 geretourneerd. De timeout van 10 ms is experimenteel gekozen.

C++:

BOOL Cusbhid::ReadBuffer(t_report* Preport)
{   DWORD Result;
    DWORD NumberOfBytesRead;
    unsigned char temp;
    unsigned char temp_return;

    //The first byte is the report number.
    InputReport.data[0]=0;

    if (ReadHandle != INVALID_HANDLE_VALUE)
    {   Result = ReadFile(ReadHandle, 
                            InputReport.data, 
                            Capabilities.InputReportByteLength, 
                            &NumberOfBytesRead,
                            (LPOVERLAPPED) &HIDOverlapped); 
    }

    Result = WaitForSingleObject 
                        (hEventObject, 
                        10);
 
    switch (Result)
    {
        case WAIT_OBJECT_0:
        {   //uit buffer lezen
            
            for(temp = 0; temp < Capabilities.InputReportByteLength; temp++)
                Preport->data[temp] = InputReport.data[temp];
            temp_return = true;

        break;
        }
        case WAIT_TIMEOUT:
        {   Result = CancelIo(ReadHandle);
            temp_return = false;
        break;
        }
        default:
        {   CloseHandles();
            device_detected = FALSE;
        break;
        }
    }
    ResetEvent(hEventObject);
    return temp_return;
}

Het is elke keer weer een hele toer om de communicatie werkend te krijgen, zowel bij de rs232, spi en de canbus.

[EddoH]

Ik zou eerlijk gezegd niet weten waarom het niet in een aparte thread zou kunnen werken. de main is tenslotte ook maar gewoon een thread, er is niets 'magisch' aan een 2e thread maken...
Misschien als de thread code post dat we iets vreemds aan je implementatie zien?

Verder nog wat opmerkingen over je code:
- C++ heeft gewoon een 'bool' type, waarom een char hiervoor gebruiken?
- waarom geen memcpy ipv for lus?

[carbidsjitter]

Ik zal het proberen uit te leggen hoe ik het heb geprobeerd te implementeren.

In de initialisatie heb ik het volgende staan.

C++:

HReadThread = CreateThread(NULL,0,ReadThread,&data_thread,0,NULL);
        SetThreadPriority(ReadThread,THREAD_PRIORITY_TIME_CRITICAL);

        //mutex for buffer
        MutexBuffer = CreateMutex(NULL, FALSE, NULL);

De thread.

C++:

DWORD WINAPI Cusbhid::ReadThread(LPVOID value)
{   while(1)
    {   Receive();
        counter_thread++;
    }
}

De functie die de thread aanroept. Wanneer er data op de usb ontvangen wordt en er de mutex is 'vrij' Dan wordt er data in de buffer gezet. Wanneer er wel data is, maar de mutex is niet vrij, dan word er gewacht tot de mutex vrij is.

C++:

void Cusbhid::Receive(void)
{   DWORD Result;
    DWORD NumberOfBytesRead;
    unsigned char temp;

    DWORD ResultWaitMutex;
    
    //The first byte is the report number.
    InputReport.data[0]=0;

    if (ReadHandle != INVALID_HANDLE_VALUE)
    {   Result = ReadFile(ReadHandle, 
                            InputReport.data, 
                            Capabilities.InputReportByteLength, 
                            &NumberOfBytesRead,
                            (LPOVERLAPPED) &HIDOverlapped); 
    }
    Result = WaitForSingleObject 
                        (hEventObject, 
                        INFINITE);
 
    switch (Result)
    {
        case WAIT_OBJECT_0:
        {   //in buffer zetten
            
            ResultWaitMutex = WaitForSingleObject( 
                                                    MutexBuffer,    // handle to mutex
                                                    INFINITE);  // no time-out interval

            switch (ResultWaitMutex) 
      { // The thread got ownership of the mutex
                case WAIT_OBJECT_0:
                {
                    //test[index] = InputReport.data[2];
                        //index++;
                
                    if(buffer_receive_used < SIZE_BUFFER)
                    {   for(temp = 0; temp < Capabilities.InputReportByteLength; temp++)
                        {   buffer_receive[buffer_receive_stop].data[temp] = InputReport.data[temp];
                        }

                        buffer_receive_used++;

                        if(++buffer_receive_stop > SIZE_BUFFER)
                            buffer_receive_stop = 0;
                    }
                    // Release ownership of the mutex object
                    if (! ReleaseMutex(MutexBuffer)) 
                    { buffer_receive_stop =buffer_receive_stop; 
                            // Handle error.
                    }
                }
                break; 

            // The thread got ownership of an abandoned mutex
            // The database is in an indeterminate state
                case WAIT_ABANDONED: 
                {buffer_receive_stop=buffer_receive_stop; 
                    //return FALSE; 
                }
            }
                    break;
        }
        case WAIT_TIMEOUT:
        {   Result = CancelIo(ReadHandle);
            //CloseHandles();
            //device_detected = FALSE;
        break;
        }
        default:
        {   CloseHandles();
            device_detected = FALSE;
        break;
        }
    }
    ResetEvent(hEventObject);
}

Code om de buffer uit te lezen. Wanneer de thread niet bezig is om de buffer te vullen (die heeft dan de niet de ownership van de mutex, dan mag de buffer uitgelezen worden. Anders wordt er gewacht tot de mutex vrij is en wordt de buffer alsnog uit gelezen.

C++:

BOOL Cusbhid::ReadBuffer(t_report* Preport)
{   DWORD ResultWaitMutex;
    unsigned char temp;

    ResultWaitMutex = WaitForSingleObject( 
                                        MutexBuffer,    // handle to mutex
                                        1000);  // no time-out interval

    switch (ResultWaitMutex) 
    {   // The thread got ownership of the mutex
        case WAIT_OBJECT_0:
        {   if(buffer_receive_used)
            {   for(temp = 0; temp < Capabilities.InputReportByteLength; temp++)
                {   Preport->data[temp] = buffer_receive[buffer_receive_start].data[temp];
                }

                buffer_receive_used--;

                if(++buffer_receive_start > SIZE_BUFFER)
                    buffer_receive_start = 0;
                
                return true;
            }
            else
            { return false;
            }

            // Release ownership of the mutex object
            if (! ReleaseMutex(MutexBuffer)) 
            { buffer_receive_start = buffer_receive_start;
                    // Handle error.
            }
        }
        // The thread got ownership of an abandoned mutex
    // The database is in an indeterminate state
        case WAIT_ABANDONED: 
        {buffer_receive_start = buffer_receive_start;
            //return FALSE; 
        }
    }
}

Het 3e stuk blok code is eigenlijk dezelfde die ik hier eerder gepost heb. Zoals ik al zei heb ik niet veel ervaring met win32 dus er zullen waarschijnlijk wel wat fundamentele fouten in zitten. Bijvoorbeeld het gebruikt van Mutex, of het in een while(1) zetten in een thread.

Ik hoor graag jullie commentaar op mijn code.

[farlane]

Mij is niet duidelijk waarom je overlapped io gebruikt, en vervolgens INFINITE gaat wachten tot ie klaar is. Doe dan gewoon een blocking read, heb je ook niets met de complexiteit van overlapped io te maken.
Ik zou je willen aanraden om met een simpele constructie te beginnen ( blocking read, geen threads ) en dat langzaam uitbouwen; als het fout gaat kun je iig wat beter traceren wanneer het fout is gegaan. En 10 tegen 1 kom je er achter dat je voor zoiets helemaal geen aparte thread nodig hebt.

[carbidsjitter]

farlane schreef op woensdag 08 december 2010 @ 08:05:
Mij is niet duidelijk waarom je overlapped io gebruikt, en vervolgens INFINITE gaat wachten tot ie klaar is. Doe dan gewoon een blocking read, heb je ook niets met de complexiteit van overlapped io te maken.

Daar heb je helemaal gelijk in. Dat is wat tegenstrijdig. Ik had het read gedeelte uit een voorbeeld van het usb boek gehaald, de fout was er wat in geslopen.

farlane schreef op woensdag 08 december 2010 @ 08:05:Ik zou je willen aanraden om met een simpele constructie te beginnen ( blocking read, geen threads ) en dat langzaam uitbouwen; als het fout gaat kun je iig wat beter traceren wanneer het fout is gegaan. En 10 tegen 1 kom je er achter dat je voor zoiets helemaal geen aparte thread nodig hebt.

Dit ben ik ook wel met je eens. Ik zie op het laatst de bomen door bos niet meer. Ik heb vandaag vrij, dus ik zal kijken of ik het goed werkend kan krijgen.

[carbidsjitter]

MLM schreef op maandag 06 december 2010 @ 23:44:
Als je dat al doet (lijkt wel wat op jouw beschrijving), dan denk ik eerder dat je USB device niet goed de data aanlevert aan windows

Dit was dus het geval. In de software van de slave (een atmel processor) kon ik de polling tijd nog instellen. Deze heb ik op 1 ms gezet. Ook gebruik ik meer bytes van de bandbreedte van het hid protocol. Ik verwerk nu 5 canbus berichten (= 55 bytes) per ms. Dit is voor mijn canbus baudrate eerst genoeg.

Iedereen bedankt voor het meedenken.

Voor degene die ook ooit een keer en hid device moeten aan sturen, heb ik de code op internet gezet. Mocht je opmerkingen hebben dan hoor ik het graag.

ps. error handling en becommentariering is nog niet helemaal klaar. Er kunnen ook nog tijdelijke variabelen in staan die ik gebruikt heb om te debuggen.

http://www.uploadarchief.net/files/download/usbhid.zip

[Rutix]

Gebruik aub geen array's in C++. Arrays zijn evil! Maak gebruik van container classes.
Ik snap dat omdat je uit C wereldje komt dat je snel naar de array wil grijpen maar gebruik de container classes (std::map,std::vector,std::string ect.) .

Oude maar goede source waarom arrays evul zijn:
http://www.parashift.com/c++-faq-lite/containers.html

[farlane]

Rutix schreef op zondag 19 december 2010 @ 19:28:
Gebruik aub geen array's in C++. Arrays zijn evil!

Lekker genuanceerd. Waarom zouden arrays eveil zijn in C++, omdat je er buiten kunt indexeren? Als je dat niet wilt gebruik je toch lekker VB.NET.

[Soultaker]

Ook een std::vector<> kun je out-of-bounds indexeren (als je operator[] i.p.v. at() gebruikt, wat iedereen altijd doet (voor bepaalde waarden van "iedereen" en "altijd")).

En zelfs de C++ FAQ zegt:

Things labeled as "evil" (macros, arrays, pointers, etc.) aren't always bad in all situations. When they are the "least bad" of the alternatives, use them!

Ik zie zelf niet echt het probleem met het gebruik van arrays zoals die in de code van carbidsjitter voorkomen. Dus tenzij Rutix z'n punt weet te onderbouwen, zou ik 'm in dit geval gewoon negeren.

[Rutix]

farlane schreef op zondag 19 december 2010 @ 23:14:
[...]

Lekker genuanceerd. Waarom zouden arrays eveil zijn in C++, omdat je er buiten kunt indexeren? Als je dat niet wilt gebruik je toch lekker VB.NET.

Nee arrays zijn evil omdat ze niet zelf bijhouden wat hun grootte is. Dat moet jij als programmeur zelf doen, en container classes zoals vectors hebben gewoon .sizes() . Ook kun je de size van een array nooit veranderen en dat kan met vector wel.

In stroustrup woorden:

In terms of time and space, an array is just about the optimal construct for accessing a sequence of objects in memory. It is, however, also a very low level data structure with a vast potential for misuse and errors and in essentially all cases there are better alternatives. By "better" I mean easier to write, easier to read, less error prone, and as fast.

The two fundamental problems with arrays are that

* an array doesn't know its own size
* the name of an array converts to a pointer to its first element at the slightest provocation

Ik zeg niet dat je nooit array's moet gebruiken maar als je het kan vermijden (wat bijna altijd het geval is.) moet je dat doen. De container classes zijn een veel beter alternatief. En als je toch array's wilt gebruiken gebruik dan tenminste boost::array.

@Soultaker:
Die comment van de C++ FAQ gaat hier niet op, omdat het hier niet de "least bad" alternatief is.

Daarnaast was mijn opmerking meer als tip voor carbidsjitter om betere C++ code te schrijven.

[ Voor 8% gewijzigd door Rutix op 20-12-2010 10:07 ]

[MLM]

Arrays zijn misschien niet zo ez-mode, maar ze hebben wel voordelen door hun low-level-heid. En je kan best een aantal van de STL algoritmes gewoon op arrays gebruiken (std::sort etc), gezien pointers ook als iterators kunnen worden gebruikt.

Voorbeeld: ik was een file aan het parsen, en die had ik in memory geladen in zijn geheel. Er zaten een aantal arrays van structures in. Om die in een std::vector te stoppen had ik de data moeten kopieren. Om ze gewoon als array te gebruiken in memory, hoefde ik niets te doen

[MSalters]

De nt kernel gebruikt een packet processing architectuur (IRP's), geen threads. Dat is primair een user-mode concept. Ik ben er dus van overtuigd dat threads NIET de oorzaak zijn.

[farlane]

Rutix schreef op maandag 20 december 2010 @ 09:59:
[...]

Dat is in ieder geval genuanceerder dan je eerste post.

[Rutix]

farlane schreef op dinsdag 21 december 2010 @ 00:43:
[...]

Dat is in ieder geval genuanceerder dan je eerste post.

Ja had later ook door dat me post beetje nutteloos leek

[epic007]

carbidsjitter schreef op dinsdag 07 december 2010 @ 23:03:
Ik hoor graag jullie commentaar op mijn code.

Ik vind je coding style redelijk onleesbaar (maar dat is smaak! ) en de constructie

C++:

buffer_receive_stop = buffer_receive_stop;

snapte ik niet helemaal.

Als je met threads werkt zou je een thread safe queue kunnen gebruiken om de data van de lees thread naar de verwerk thread te krijgen.
Zoiets:

C++:

Queue queue;

void UsbReadThread()
{
  USB usb;
  while (!quit) {
    data = usb.read(...);
    queue.enqueue(data);
  }
}

void DataProcessThread()
{
  while(!quit) {
    data = queue.dequeue();
    ProcessTheData(data);
  }
}

De Queue classe doet intern aan thread synchronisatie door een Mutex of CriticalSection o.i.d.

[carbidsjitter]

epic007 schreef op dinsdag 21 december 2010 @ 22:26:
[...]Ik vind je coding style redelijk onleesbaar (maar dat is smaak! )

Kun je een voorbeeld geven van hoe het eventueel beter zou kunnen ?

C++:

buffer_receive_stop = buffer_receive_stop;

Vergeten weg te halen, dit gebruikte ik om een debug punt te 'maken'.

Inderdaad, in de c wereld gebruik ik vaak array's en de fouten die jullie noemen (vooral buiten de gedeclareerde index gaan van de array) is meermaals voor gekomen. Als ik deze fouten gemakkelijker kan voorkomen zal ik dat zeker proberen. Ik heb hier soms rare situaties mee gehad, bijvoorbeeld dat heel veel variabelen in eens andere waarden heeft. (Ik stuur soms met mijn software grote machine's aan, dus fouten zijn uit den boze)

Die Queue variabele ziet er ook netjes uit.

Ik had niet zoveel reacties verwacht, ik zal proberen de code te optimaliseren en dan zet ik het hier weer neer. Dan mogen jullie mij weer afbranden

[Mijzelf]

carbidsjitter schreef op woensdag 22 december 2010 @ 09:48:
Inderdaad, in de c wereld gebruik ik vaak array's en de fouten die jullie noemen (vooral buiten de gedeclareerde index gaan van de array) is meermaals voor gekomen. Als ik deze fouten gemakkelijker kan voorkomen zal ik dat zeker proberen.

Ik heb voor de bewaking van arrays wel dit soort code gebruikt:

code:

#define ARRAY_ELEMENT(array,index)  array[index], assert(index>=0 && index<(sizeof(array)/sizeof(array[0]))), array[index]

void Test()
{
    int buffer[ 5 ] = { 0, 1, 2, 3, 4 };
    int i;

    i = ARRAY_ELEMENT(buffer,4);
    ARRAY_ELEMENT(buffer,3) = 7;
    
    i = ARRAY_ELEMENT(buffer,9); /* Assertion failure */
}

Werkt natuurlijk alleen met 'echte' arrays, en niet met pointers.

[farlane]

carbidsjitter schreef op woensdag 22 december 2010 @ 09:48:
Inderdaad, in de c wereld gebruik ik vaak array's en de fouten die jullie noemen (vooral buiten de gedeclareerde index gaan van de array) is meermaals voor gekomen.

Die bug *kan* voorkomen in voor mij elk bekende taal die arrays heeft ( alle? ), het resultaat van een violation verschilt alleen van reboot tot DrWatson tot irritante popup tot format van je C: drive.

[Soultaker]

Rutix schreef op maandag 20 december 2010 @ 09:59:
Nee arrays zijn evil omdat ze niet zelf bijhouden wat hun grootte is.

Onzin. Een array heeft gewoon een grootte.

Ook kun je de size van een array nooit veranderen en dat kan met vector wel.

Dat is alleen een nadeel als je de grootte wil kunnen veranderen. Voor sommige doeleinden is dat nuttig (en dan is std::vector<> inderdaad een uitkomst) maar in dit geval niet omdat de buffergrootte toch al een runtimeconstante is.

Het is mijns inziens veel zinniger om een simpele constructie te gebruiken als die volstaat.

Ik zeg niet dat je nooit array's moet gebruiken maar als je het kan vermijden (wat bijna altijd het geval is.) moet je dat doen.

Waarom? Waarom niet een simpeler alternatief gebruiken als dat aan alle eisen voldoet? std::vector<> heeft voordelen én nadelen; als je in een specifieke situatie van geen van de voordelen kunt profiteren waarom zou je dan de nadelen erbij moeten nemen?

En als je toch array's wilt gebruiken gebruik dan tenminste boost::array.

Ja, dat is helemaal een goed advies. Laten we een 40MB library dependency introduceren, niet omdat die in het onderhavige geval enige functionaliteit toevoegt, maar puur zodat we niet van een standaard built-in taalfeature gebruik hoeven maken. Als je al boost gebruikt als platform voor ontwikkeling is het een ander verhaal natuurlijk.

Misschien moet je maar als softwareconsultant aan de slag, dan kun je grote bedrijven adviseren over Enterpise™ Software® Development© best-practices. Dan kun je een leuke boterham verdienen met doeltreffende code onnodig ingewikkeld maken.

[.oisyn]

Ik heb het échte argument om std::vector te gebruiken boven arrays nog niet gehoord: exception safety. Gaat uiteraard alleen op bij runtime gealloceerde arrays.

Mijzelf schreef op woensdag 22 december 2010 @ 12:37:
[...]


Ik heb voor de bewaking van arrays wel dit soort code gebruikt:

code:

#define ARRAY_ELEMENT(array,index)  array[index], assert(index>=0 && index<(sizeof(array)/sizeof(array[0]))), array[index]

void Test()
{
    int buffer[ 5 ] = { 0, 1, 2, 3, 4 };
    int i;

    i = ARRAY_ELEMENT(buffer,4);
    ARRAY_ELEMENT(buffer,3) = 7;
    
    i = ARRAY_ELEMENT(buffer,9); /* Assertion failure */
}

Werkt natuurlijk alleen met 'echte' arrays, en niet met pointers.

Als het C++ is, dan:

C++:

template<class T, size_t N>
T & ARRAY_ELEMENT(T (&arr)[N], size_t index)
{
    assert(index < N);
    return arr[index];
}

template<class T, size_t N>
const T & ARRAY_ELEMENT(const T (&arr)[N], size_t index)
{
    assert(index < N);
    return arr[index];
}

Jouw macro kan al crashen voordat de assert wordt aangeroepen. Ook werkt hij niet als je het resultaat als functieparameter wil gebruiken. Áls je dan echt de macro wilt gebruiken, zou ik 'm nog altijd zo implementeren:

C++:

#define ARRAY_ELEMENT(array,index)  (assert(index>=0 && index<(sizeof(array)/sizeof(array[0]))), array[index])

[ Voor 87% gewijzigd door .oisyn op 23-12-2010 01:12 ]

[Soultaker]

Die macro heeft als nadeel dat het in C niet naar een lvalue evalueert, waardoor je er niet aan kunt assignen. In C++ evalueert het naar een reference, waardoor dat wel kan, maar zoals je zelf al zei is in C++ een inline template functie toch te prefereren. Dus de macro zoals jij 'm definieert lijkt me niet praktisch.

Deze variant neemt beide bezwaren weg, denk ik:

C:

#define ARRAY_ELEMENT(array,index)  array[assert((size_t)(index) < sizeof(array)/sizeof(*array)), (index)]

Alle genoemde macros hebben nog wel het nadeel dat ze index meer dan eens evalueren (tenzij NDEBUG gedefinieerd is natuurlijk). Dat kan opgelost worden door een hulpfunctie te definiëren:

C:

static size_t check_index(size_t index, size_t length) {
    assert(index < length);
    return index;
}

#define ARRAY_ELEMENT(array,index) array[check_index(index, sizeof(array)/sizeof(*array))]

In op GCC-gebaseerde systemen worden vaak GCC's statement expressies gebruikt voor dit soort dingen:

C:

#define ARRAY_ELEMENT(array,index) array[({ size_t i = (index); assert(i < sizeof(array)/sizeof(*array)); i; })]

Dit heeft als voordeel dat je geen hulpfunctie hoeft te introduceren, en je assertion triggert gratis op de regel waar de macro gebruikt wordt in plaats van in je hulpfunctie. Maar dit is natuurlijk geen standaard C.

[.oisyn]

Soultaker schreef op donderdag 23 december 2010 @ 01:41:
Die macro heeft als nadeel dat het in C niet naar een lvalue evalueert

Ah, right . C sucks

[carbidsjitter]

Het volgende probleem:

Omdat ik de usb naar canbus omzetter vaak in het 'veld' zal gebuiken om microcontroller te updaten via de canbus, heb ik de applicatie op de laptop gebruikt.

Tot mijn verbazing verzend de applicatie 10 x zo langzaam als mijn vaste computer, dus in plaats van 1 ms, 10 ms.

Nu heb ik met het volgende stukje code de timing gemeten van de uitvoer van de code en het verzenden via de canbus.

C++:

//globale variabelen
LARGE_INTEGER begin_count;
LARGE_INTEGER end_count;
LARGE_INTEGER frequency;
unsigned short time_begin_end_micro;
unsigned short time_end_begin_micro;
unsigned short table_begin_end[10000];
unsigned short table_end_begin[10000];
unsigned short index;

//code om de tijd te meten
QueryPerformanceFrequency(&frequency);

while(1)
{

QueryPerformanceCounter(&begin_count);
//tijd voor code
time_end_begin_micro = (((unsigned long long)begin_count.LowPart - end_count.LowPart) * 1000000) / frequency.LowPart;

    if (WriteHandle != INVALID_HANDLE_VALUE)
    {   Result = WriteFile 
        (WriteHandle, 
        OutputReport_temp.data, 
        Capabilities.OutputReportByteLength, 
        &BytesWritten, 
        NULL);
    }
    
QueryPerformanceCounter(&end_count);
//tijd om data weg te schrijven
time_begin_end_micro = (((unsigned long long)end_count.LowPart - begin_count.LowPart) * 1000000) / frequency.LowPart;

//tijden gemeten in microseconden
table_begin_end[index] = time_begin_end_micro;
table_end_begin[index] = time_end_begin_micro;
if(++index == 10000)
    index = 0;

Voor de meting van timing gebruik ik 2 tabellen. Eentje om de tijdsduur van de WriteFile commando te meten en eentje om de tijdsduur van de uitvoering van de code te meten. het maakt vrij weinig uit of ik dit stukje in een while loop zet of niet. De code wordt snel uitgevoerd, behalve de writefile.

In de tabel op de vaste computer staat continue 1000 in dus 1 ms en op de laptop 10000 (10 ms).

Ik heb een vermoeden dat op een of andere manier komt door de OS en de tijdsverdeling aan de usb poorten.

Heeft iemand een idee wat de oorzaak van dit probleem is en hoe ik dit kan verhelpen ?

[farlane]

Erm, wat is er aan de hand met het QuadPart dat je die niet wilt gebruiken?

Het kan natuurlijk zijn dat je brakke drivers hebt, maar USB drivers gebruiken iha best grote buffers en DMA om de data naar de pheripherals te krijgen, dus de kernel is aan de transfer zelf waarschijnlijk niet zo heel veel tijd kwijt.

[ Voor 61% gewijzigd door farlane op 05-01-2011 12:18 ]

[Niemand_Anders]

Er vanuit gaand dat je tweede dezelfde type USB poorten vergelijkt (USB 1.0, USB 2.0, USB 3.0, etc) kunnen er natuurlijk ook (grote) verschillen in de gebruikte USB chipsets zitten. Laptops gooien veel processen in een lagere versnelling om zo energie te besparen. Zelfs een budget en een high-end moederbord laten al grote verschillen zien..

[MacGrumpy]

Windows werkt met een ring buffer. Als je vaker leest dan dat de buffer gevuld word zie je oude waarden. Lees je langzamer, mis je waardes. (zie http://msdn.microsoft.com/en-us/library/ms923752.aspx)
De groote van deze ringbuffer is in te stellen, onder windows 7 was hij (uit eigen ervaring) ongeveer 100 groot.
Het instellen van de buffer word hier beschreven: http://msdn.microsoft.com...f539686%28v=VS.85%29.aspx.

Maar houd er rekening mee dat je op een niet RT systeem geen garantie hebt dat je iedere X ms iets kan uitlezen, je code moet dus robust zijn tegen ontbrekende hidreports.

p.s. Zelf heb ik ook problemen gehad met GetInputReport, en was ook genoodzaakt ReadFile te gebruiken.

[MLM]

Misschien dat USB I/O maar op 100Hz draait? Lijkt me niet onwaarschijnlijk op een laptop in energysaver mode.

Dat WriteFile lang duurt is logisch, het is immers een blocking call (het vraagt het OS om de data te versturen, en wacht dan tot het al dan niet gelukt is). Als je kijkt naar CPU verbruik van je proces zal je waarschijnlijk zien dat dat <1% is, omdat je thread gewoon idlet tijdens WriteFile.

Als je niet wilt dat je thread idlet moet je asynchronous I/O gebruiken (het vraagt het OS om de data te versturen, maar wacht NIET tot het al dan niet gelukt is, dat kan je "later" doen), maar ik denk niet dat je meer throughput gaat krijgen ofzo, dit lijkt op een limitatie van de USB driver/stack, en wat jij ook doet vanuit usermode, dat gaat niet sneller gaan

[farlane]

MLM schreef op woensdag 05 januari 2011 @ 13:28:
Dat WriteFile lang duurt is logisch, het is immers een blocking call (het vraagt het OS om de data te versturen, en wacht dan tot het al dan niet gelukt is).

Niet tot het verstuurd is, maar totdat het gebufferd is in de driver/kernel. Tenminste, dat is normaal gesproken het geval.

[carbidsjitter]

farlane schreef op woensdag 05 januari 2011 @ 12:14:
Erm, wat is er aan de hand met het QuadPart dat je die niet wilt gebruiken?

Voorbeeldje van internet gebruikt.

Het device wat ik aan stuur wordt zowel op de laptop als vaste computer aangemeld als usb1.1 device.

@MacGrumpy
Het gaat hier om het zenden. Ontvangen heb ik net getest met dezelfde manier als ik het verzenden ook getest heb. Het vreemde is dat ik bij het ontvangen wel een interval heb van 1 ms, zowel op de laptop als vaste computer.

Ook heb ik de energiebeheer op performance gezet, laptop zit ook aan de adapter. Dit maakt verder geen verschil.

De volgende link is hoe het device is aangemeld, bekeken met usb view

http://www.uploadarchief....ownload/usbviewlaptop.png

Daar staat bij interval wel 1 (is dus 1ms)

Het blijft wat een vreemd probleem.

[edit]
Laptop = windows 7 home premium, vaste computer = windows 7 proffesional

[ Voor 4% gewijzigd door carbidsjitter op 05-01-2011 14:47 ]

[.oisyn]

carbidsjitter schreef op woensdag 05 januari 2011 @ 14:39:
[...]

Voorbeeldje van internet gebruikt.

Dat voorbeeldje klopt dus niet. Ik prefereer zelf gewoon long long (of __int64), waarvan ik het adres dan cast naar een LARGE_INTEGER* als ik 'm aan een van die functies geef. Dan kun je er gewoon mee rekenen.

C++:

long long GetPerformanceFrequency()
{
    long long r;
    QueryPerformanceFrequency((LARGE_INTEGER*)&r);
    return r;
}

// en hetzelfde voor QueryPerformanceCounter()

[MLM]

farlane schreef op woensdag 05 januari 2011 @ 13:34:
[...]

Niet tot het verstuurd is, maar totdat het gebufferd is in de driver/kernel. Tenminste, dat is normaal gesproken het geval.

Aangezien dat het punt is waar je weet of je I/O gelukt is of niet. Verstuurd is een beetje krom, maar WriteFile werkt op meer dan alleen files (bijvoorbeeld, een USB device, socket, pipe etc), en in de context van het USB device vond ik "verstuurd" een redelijke werkwoord keus

[farlane]

.oisyn schreef op woensdag 05 januari 2011 @ 14:58:
[...]

Dat voorbeeldje klopt dus niet. Ik prefereer zelf gewoon long long (of __int64), waarvan ik het adres dan cast naar een LARGE_INTEGER* als ik 'm aan een van die functies geef. Dan kun je er gewoon mee rekenen.

C++:

long long GetPerformanceFrequency()
{
    long long r;
    QueryPerformanceFrequency((LARGE_INTEGER*)&r);
    return r;
}

// en hetzelfde voor QueryPerformanceCounter()

Waarom niet netjes QuadPart gebruiken?

The LARGE_INTEGER structure is actually a union. If your compiler has built-in support for 64-bit integers, use the QuadPart member to store the 64-bit integer.

[farlane]

MLM schreef op woensdag 05 januari 2011 @ 15:29:
Aangezien dat het punt is waar je weet of je I/O gelukt is of niet. Verstuurd is een beetje krom, maar WriteFile werkt op meer dan alleen files (bijvoorbeeld, een USB device, socket, pipe etc), en in de context van het USB device vond ik "verstuurd" een redelijke werkwoord keus

Mja maar er is een verschil tussen 'gebufferd door de driver' en 'verstuurd op de bus', wat in sommige gevallen behoorlijk relevant is. Bv flushen van je output buffers net nadat je WriteFile klaar was.

[.oisyn]

farlane schreef op woensdag 05 januari 2011 @ 16:27:
Waarom niet netjes QuadPart gebruiken?

Omdat ik het irritant vind om al m'n timers als LARGE_INTEGER te typeren en continu .QuadPart erachter moet typen. Natuurlijk, in mijn korte voorbeeld kan dat wel, maar over het algemeen typ ik direct de win32 API functie ipv dat ik een wrapper gebruik.

[farlane]

.oisyn schreef op woensdag 05 januari 2011 @ 18:00:
Omdat ik het irritant vind om al m'n timers als LARGE_INTEGER te typeren en continu .QuadPart erachter moet typen. Natuurlijk, in mijn korte voorbeeld kan dat wel, maar over het algemeen typ ik direct de win32 API functie ipv dat ik een wrapper gebruik.

Ah ok dat kan ik me voorstellen, mijn vraag ging over je voorbeeld.