Hi there,

I'm developing an audio device based on a cubieboard2(armf). I use potentiometers read by an adc that communicates through i²c protocol with my cubie. An i²c lcd display shows the desired parameters values.

I wrote externals in order to get data from the potentiometers (and other switches, and a rotary encoder), or send data to the display. Everything was working, but the i²c I/O functions calls were leading to clicks 'n pops in the audio, which was unacceptable. I use a rt patched debian with selected rt priorities for irqs and audio as I always did with success, and I am looking for a smart way to make my pd patches communicate with my physical interface, in a transparent way at audio level.

Then a opened this topic : http://forum.pdpatchrepo.info/topic/9489/external-i2c-data-reader-leads-to-clicks-standalone-version-works-better , and @Eeight proposed to implement threads, kindly giving a template in order to show me the way.

And now I'd need some advice. I'm not a real C programmer, just learning the empirical way, and I'm reaching my limits... I made a threaded version of the external that reads the potentiometers every x milliseconds, and it works perfectly, no audio pollution anymore. But when I made a threaded version of the external that handles the i²c display (thus receiving incoming messages such as "position x y", or "write message"), it lead to timing problems.

Some of the messages get uninterpreted because my writing function is threaded, and takes the form of an infinite while(1) loop containing a "usleep(10000)" instruction at the end of it in order to limit cpu load. The problem is that this leads to the "loss" of most of the incoming messages...

When reading potentiometers values, it's ok to get them merely one every 10ms, but when you have to send messages that can be interpreted merely one every 1oms, you have to send them to the external with delays, which works (That's my present situation), but is tiedious and inelegant.

Would someone have an idea of how this problem could be addressed in a more elegant manner ?

Here is how I implemented it :

• I create a thread for an infinite while (1) loop containing a call to a writing function whenever a flag equals 1, followed by a usleep(10000) instruction.
  This "thread" is running from the beginning and awaits for a nonzero value of the flag to enter in action. It reads the string to be displayed from the object's data structure, but only when the "clocking" allows it together with the flag
• a "write" method can receive a t_symbol : the string to display. When a "write blahblah" message is received by the object, the string "blahblah" is stored in the object's data structure, as the flag which is set to 1.
• Then the next time the threaded loop evaluates the flag, it displays "blahblah", resets the flag to 0 and sleeps for 10ms.
  But when you use incoming messages using 10 lines messages (allowing for cursor positioning and writing orders), they of course flow through the code in far less than 10ms, hence my problems... In other words the string to be displayed can be changed several times in the object's data structure without being actually displayed because of the relatively slow "clocking".

Please forget the incredible length of this message, together to the fact that I don't post the source yet because of a basic shame of my inelegant coding style. Maybe will I finally clean it and post it later.

Thank you,

Nau

@nau Hi, same boat (I don't know much about Pd internal functions & pthread), but maybe you can try to see if this external (really similar to my template, but this time to fetch real data for my HiCu project).

Look for m_clock / m_interval and clock_delay.

// ==============================================================================
//  gac.c
//  
//  pd-Interface to [ 11h11 | gac ]
//  Adapted by: Patrick Sebastien Coulombe
//  Website:    http://www.workinprogress.ca/guitare-a-crayon
//
//  Original Author:    Michael Egger
//  Copyright:  2007 [ a n y m a ]
//  Website:    http://gnusb.sourceforge.net/
//  
//  License:    GNU GPL 2.0 www.gnu.org
//  Version:    2009-04-11
// ==============================================================================
// ==============================================================================

#include "m_pd.h"
#include <usb.h> //http://libusb-win32.sourceforge.net
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include "pthread.h"

#include "../common/gac_cmds.h"


// ==============================================================================
// Constants
// ------------------------------------------------------------------------------
#define USBDEV_SHARED_VENDOR        0x16c0  /* VOTI */
#define USBDEV_SHARED_PRODUCT       0x05dc  /* Obdev's free shared PID */
#define DEFAULT_CLOCK_INTERVAL      34      /* ms */
#define OUTLETS                     11
#define USBREPLYBUFFER              14

unsigned char buffer[USBREPLYBUFFER]; //accessible everywhere

// ==============================================================================
// Our External's Memory structure
// ------------------------------------------------------------------------------
typedef struct _gac             // defines our object's internal variables for each instance in a patch
{
    t_object        p_ob;                   // object header - ALL pd external MUST begin with this...
    usb_dev_handle  *dev_handle;            // handle to the gac usb device
    void            *m_clock;               // handle to our clock
    double          m_interval;             // clock interval for polling edubeat
    double          m_interval_bak;         // backup clock interval for polling edubeat
    int             is_running;             // is our clock ticking?
    void            *outlets[OUTLETS];      // handle to the objects outlets
    int             x_verbose;
    pthread_attr_t  gac_thread_attr;
    pthread_t       x_threadid;
} t_gac;

void *gac_class;                    // global pointer to the object class - so pd can reference the object 


// ==============================================================================
// Function Prototypes
// ------------------------------------------------------------------------------
void *gac_new(t_symbol *s);
void gac_assist(t_gac *x, void *b, long m, long a, char *s);
void gac_bang(t_gac *x);                
void gac_bootloader(t_gac *x);

static int usbGetStringAscii(usb_dev_handle *dev, int ndex, int langid, char *buf, int buflen);
void find_device(t_gac *x);

// =============================================================================
// Threading
// ------------------------------------------------------------------------------
static void *usb_thread_read(void *w)
{
    t_gac *x = (t_gac*) w;
    int nBytes;
    
    while(1) {
        pthread_testcancel();
            if (!(x->dev_handle)) find_device(x);
            else {
                    nBytes = usb_control_msg(x->dev_handle, USB_TYPE_VENDOR | USB_RECIP_DEVICE | USB_ENDPOINT_IN, 
                                        EDUBEAT_CMD_POLL, 0, 0, (char *)buffer, sizeof(buffer), DEFAULT_CLOCK_INTERVAL);
                    if(x->x_verbose)post("thread read %i bytes", nBytes);
                    //post("%i b", nBytes);
            }
    }
    return 0;
}

static void usb_thread_start(t_gac *x) {
    // create the worker thread
    if(pthread_attr_init(&x->gac_thread_attr) < 0)
    {
       error("gac: could not launch receive thread");
       return;
    }
    if(pthread_attr_setdetachstate(&x->gac_thread_attr, PTHREAD_CREATE_DETACHED) < 0)
    {
       error("gac: could not launch receive thread");
       return;
    }
    if(pthread_create(&x->x_threadid, &x->gac_thread_attr, usb_thread_read, x) < 0)
    {
       error("gac: could not launch receive thread");
       return;
    }
    else
    {
       if(x->x_verbose)post("gac: thread %d launched", (int)x->x_threadid );
    }
}

//--------------------------------------------------------------------------
// - Message: bootloader
//--------------------------------------------------------------------------
void gac_bootloader(t_gac *x)
{
    int cmd;
    int nBytes;
    unsigned char bootloaderbuffer[8];
    
    cmd = 0;
    
    cmd = EDUBEAT_CMD_START_BOOTLOADER;
    if (!(x->dev_handle)) find_device(x);
    else {
        nBytes = usb_control_msg(x->dev_handle, USB_TYPE_VENDOR | USB_RECIP_DEVICE | USB_ENDPOINT_IN, 
                            cmd, 0, 0, (char *)bootloaderbuffer, sizeof(bootloaderbuffer), DEFAULT_CLOCK_INTERVAL);
    }
}

//--------------------------------------------------------------------------
// - Message: bang  -> poll gac
//--------------------------------------------------------------------------
void gac_bang(t_gac *x) {
    int                 i,n;
    int                 replymask,replyshift,replybyte;
    int                 temp;
    
    for (i = 0; i < OUTLETS; i++) {
        temp = buffer[i];
        
        switch(i) {
            case 0:
                replybyte = buffer[8];
                replyshift = ((0 % 4) * 2);
                replymask = (3 << replyshift);
                temp = temp * 4 + ((replybyte & replymask) >> replyshift);
            break;
            case 1:
                replybyte = buffer[8];
                replyshift = ((1 % 4) * 2);
                replymask = (3 << replyshift);
                temp = temp * 4 + ((replybyte & replymask) >> replyshift);
            break;
            case 2:
                replybyte = buffer[8];
                replyshift = ((2 % 4) * 2);
                replymask = (3 << replyshift);
                temp = temp * 4 + ((replybyte & replymask) >> replyshift);
            break;
            case 3:
                replybyte = buffer[8];
                replyshift = ((3 % 4) * 2);
                replymask = (3 << replyshift);
                temp = temp * 4 + ((replybyte & replymask) >> replyshift);
            break;
            case 4:
                replybyte = buffer[9];
                replyshift = ((0 % 4) * 2);
                replymask = (3 << replyshift);
                temp = temp * 4 + ((replybyte & replymask) >> replyshift);
            break;
            case 5:
                replybyte = buffer[9];
                replyshift = ((1 % 4) * 2);
                replymask = (3 << replyshift);
                temp = temp * 4 + ((replybyte & replymask) >> replyshift);
            break;
            case 6:
                replybyte = buffer[9];
                replyshift = ((2 % 4) * 2);
                replymask = (3 << replyshift);
                temp = temp * 4 + ((replybyte & replymask) >> replyshift);
            break;
            
            case 8:
                temp = buffer[10];
                replybyte = buffer[13];
                replyshift = ((0 % 4) * 2);
                replymask = (3 << replyshift);
                temp = temp * 4 + ((replybyte & replymask) >> replyshift);
            break;
            case 9:
                temp = buffer[11];
                replybyte = buffer[13];
                replyshift = ((1 % 4) * 2);
                replymask = (3 << replyshift);
                temp = temp * 4 + ((replybyte & replymask) >> replyshift);
            break;
            case 10:
                temp = buffer[12];
                replybyte = buffer[13];
                replyshift = ((2 % 4) * 2);
                replymask = (3 << replyshift);
                temp = temp * 4 + ((replybyte & replymask) >> replyshift);
            break;
        }
        outlet_float(x->outlets[i], temp);
    }
            
}

//--------------------------------------------------------------------------
// - The clock is ticking, tic, tac...
//--------------------------------------------------------------------------
void gac_tick(t_gac *x) { 
    clock_delay(x->m_clock, x->m_interval);     // schedule another tick
    gac_bang(x);                                // poll the edubeat
} 


//--------------------------------------------------------------------------
// - Object creation and setup
//--------------------------------------------------------------------------
int gac_setup(void)
{
    gac_class = class_new ( gensym("gac"),(t_newmethod)gac_new, 0, sizeof(t_gac),   CLASS_DEFAULT,0);
    // Add message handlers
    class_addbang(gac_class, (t_method)gac_bang);
    class_addmethod(gac_class, (t_method)gac_bootloader, gensym("bootloader"), A_DEFSYM,0);
    post("bald-approved gac version 0.1",0);
    return 1;
}

//--------------------------------------------------------------------------
void *gac_new(t_symbol *s)      // s = optional argument typed into object box (A_SYM) -- defaults to 0 if no args are typed
{
    t_gac *x;                                   // local variable (pointer to a t_gac data structure)
    x = (t_gac *)pd_new(gac_class);          // create a new instance of this object
    x->m_clock = clock_new(x,(t_method)gac_tick);
    x->x_verbose = 0;

    x->m_interval = DEFAULT_CLOCK_INTERVAL;
    x->m_interval_bak = DEFAULT_CLOCK_INTERVAL;
    
    x->dev_handle = NULL;
    int i;
    
    // create outlets and assign it to our outlet variable in the instance's data structure
    for (i=0; i < OUTLETS; i++) {
        x->outlets[i] = outlet_new(&x->p_ob, &s_float);
    }   

    usb_thread_start(x); //start polling the device
    clock_delay(x->m_clock,0.); //start reading the buffer

    return x; // return a reference to the object instance 
}

//--------------------------------------------------------------------------
// - Object destruction
//--------------------------------------------------------------------------
void gac_free(t_gac *x)
{
    if (x->dev_handle) usb_close(x->dev_handle);
    freebytes((t_object *)x->m_clock, sizeof(x->m_clock));
        while(pthread_cancel(x->x_threadid) < 0)
            if(x->x_verbose)post("gac: killing thread\n");
        if(x->x_verbose)post("gac: thread canceled\n");
            
}

//--------------------------------------------------------------------------
// - USB Utility Functions
//--------------------------------------------------------------------------
static int  usbGetStringAscii(usb_dev_handle *dev, int ndex, int langid, char *buf, int buflen)
{
char    asciibuffer[256];
int     rval, i;

    if((rval = usb_control_msg(dev, USB_ENDPOINT_IN, USB_REQ_GET_DESCRIPTOR, (USB_DT_STRING << 8) + ndex, langid, asciibuffer, sizeof(asciibuffer), 1000)) < 0)
        return rval;
    if(asciibuffer[1] != USB_DT_STRING)
        return 0;
    if((unsigned char)asciibuffer[0] < rval)
        rval = (unsigned char)asciibuffer[0];
    rval /= 2;
    /* lossy conversion to ISO Latin1 */
    for(i=1;i<rval;i++){
        if(i > buflen)  /* destination buffer overflow */
            break;
        buf[i-1] = asciibuffer[2 * i];
        if(asciibuffer[2 * i + 1] != 0)  /* outside of ISO Latin1 range */
            buf[i-1] = '?';
    }
    buf[i-1] = 0;
    return i-1;
}

//--------------------------------------------------------------------------
void find_device(t_gac *x) {
    usb_dev_handle      *handle = NULL;
    struct usb_bus      *bus;
    struct usb_device   *dev;
    
    usb_init();
    usb_find_busses();
    usb_find_devices();
     for(bus=usb_busses; bus; bus=bus->next){
        for(dev=bus->devices; dev; dev=dev->next){
            if(dev->descriptor.idVendor == USBDEV_SHARED_VENDOR && dev->descriptor.idProduct == USBDEV_SHARED_PRODUCT){
                char    string[256];
                int     len;
                handle = usb_open(dev); /* we need to open the device in order to query strings */
                if(!handle){
                    error ("Warning: cannot open USB device: %s", usb_strerror());
                    continue;
                }
                /* now find out whether the device actually is gac */
                len = usbGetStringAscii(handle, dev->descriptor.iManufacturer, 0x0409, string, sizeof(string));
                if(len < 0){
                    post("gac: warning: cannot query manufacturer for device: %s", usb_strerror());
                    goto skipDevice;
                }
                post("::::::%s", string);
                
                if(strcmp(string, "11h11") != 0)
                    goto skipDevice;
                len = usbGetStringAscii(handle, dev->descriptor.iProduct, 0x0409, string, sizeof(string));
                if(len < 0){
                    post("gac: warning: cannot query product for device: %s", usb_strerror());
                    goto skipDevice;
                }
                if(strcmp(string, "Gac") == 0)
                    break;
                skipDevice:
                usb_close(handle);
                handle = NULL;
            }
        }
        if(handle)
            break;
    }
    
    if(!handle){
        post("Could not find USB device 11h11/gac");
        x->dev_handle = NULL;
    } else {
         x->dev_handle = handle;
         post("Found USB device 11h11/gac");
    }
}

gac.c

Cheers

@EEight : thanks man, had a surface look at it, but first thought it didn't fit my needs... but after another pair of hours trying other ways I feel like I will have to look deeper into this last proposal...
In fact I thought I could easily make readings from my I²C ADC whithin a thread, but I noticed that I got errors in the console under peculiar conditions (when jitter make the output "density" higher), so this is not a very stable solution yet.
To be continued...
Thanks,
Nau