Feat : i1Pro 2 Visual LED Feedback in chartread #37

Closed
opened 2026-09-04 16:52:15 +01:00 by gronod · 0 comments
Owner

Target Subsystem: spectro/chartread.c, spectro/inst.h, spectro/i1pro.c, spectro/i1pro_imp.h
Supported Hardware: X-Rite i1Pro 2 (Rev E)
Deliverable Format: Markdown Technical Implementation Specification


1. Executive Summary

This document specifies the technical architecture and code modifications required to implement real-time visual status feedback via the integrated ring LEDs on the X-Rite i1Pro 2 spectrophotometer within ArgyllCMS's chartread utility.

The feature introduces a dedicated command-line flag to govern LED signaling across distinct stages of the chart-reading lifecycle:

Stage Visual Indicator
Awaiting Base Calibration Flashing White
Ready to Scan Row Flashing Blue
Row Read Failed / Scan Error Flashing Red
Row Read Succeeded Flashing Green

If the command-line flag is omitted, or if an instrument lacking programmable RGB indicators (such as an i1Pro Rev A–D, ColorMunki, or third-party colorimeter) is connected, the operational behaviour degrades silently to default ArgyllCMS mechanics without throwing errors or halting execution.


2. Command-Line Switch Selection

2.1 Audit of Existing chartread Options

An audit of spectro/chartread.c argument parsing confirms the following active options:

Flag Argument Function
-v [level] Verbose execution mode
-c port Communication port selection
-t None Transmissive chart read mode
-d None Direct display reading mode
-p None Spot / patch-by-patch mode instead of strip mode
-r None Resume an incomplete chart session
-n None Do not save incomplete chart data
-N None Suppress initial calibration prompt if possible
-B None Suppress bi-directional strip reading
-H None High-resolution spectral data collection
-F filter Filter configuration (none, pol, d65, etc.)
-T factor Patch recognition tolerance factor
-A factor Patch recognition averaging factor
-y type Display type specification
-X file.cal External calibration file override

2.2 Selected Switch: -L

The uppercase letter -L is unused in chartread and serves as a mnemonic for LED status indicators.

  • Usage Syntax: chartread -L [other_options] basename
  • Parsing Semantics: Boolean flag; requires no parameter argument.
  • Fallback Contract: When -L is passed but the attached spectrophotometer does not expose the inst_stat_leds capability, the flag is silently ignored.

3. LED Functional State Matrix

The i1Pro 2 (Rev E) features two multi-colour LEDs flanking the optical button. Both LEDs are driven concurrently to ensure 360° visibility.

Lifecycle State Target Visual Pattern Dominant Colour Duty Cycle / Timing Termination Trigger
Awaiting Calibration Dual Flash / Pulse White (#FFFFFF) 500 ms ON / 500 ms OFF (1.0 Hz) Instrument leaves calibration plate, or button/Enter is pressed.
Ready for Row Scan Dual Pulse Blue (#0000FF) 300 ms ON / 700 ms OFF (1.0 Hz) Measurement button pressed, optical sensor detects movement, or swipe initiates.
Row Read Failure Rapid Strobe Red (#FF0000) 100 ms ON / 100 ms OFF (3 pulses, 600 ms total) Automatically reverts to Ready for Row Scan (Flashing Blue) upon re-prompting.
Row Read Success Confirmation Flash Green (#00FF00) 400 ms solid illumination Automatically extinguishes, then transitions to Ready for Row Scan for next row.
Shutdown / Standby Extinguished Off (#000000) Static 0% duty cycle Invoked upon port closure, session completion, or SIGINT.

4. Architectural Design & Abstraction Layers

To maintain clean separation between Argyll's hardware-agnostic application layer (chartread.c) and device-specific driver code, the implementation is divided across two architectural boundaries:

+-------------------------------------------------------------+
|                     chartread.c                             |
|  - Parses '-L' switch into 'g_use_leds' global flag         |
|  - Calls inst->set_led_state() at state transition points   |
+-------------------------------------------------------------+
                              |
                              | (inst_led_state enum)
                              v
+-------------------------------------------------------------+
|                      inst.h / inst.c                        |
|  - Exposes inst_led_state typedef                           |
|  - Defines INST_CAP_LED_STATUS capability bitmask           |
|  - Declares inst_code (*set_led_state)(inst *p, ...)        |
+-------------------------------------------------------------+
                              |
                              | (Internal Driver Dispatch)
                              v
+-------------------------------------------------------------+
|                     spectro/i1pro.c                         |
|  - Implements i1pro2_set_led_state()                        |
|  - Manages background LED flashing thread or firmware modes |
|  - Writes 5-byte command packets to USB Endpoint 0x05       |
+-------------------------------------------------------------+

5. Driver Layer Implementation (spectro/)

5.1 Interface Extensions: spectro/inst.h

Add enumeration states and function pointer signatures to the generic instrument interface:

/* --- spectro/inst.h additions --- */

/* Generic Instrument LED Status States */
typedef enum {
    inst_led_off           = 0,
    inst_led_cal_wait      = 1, /* Flashing White */
    inst_led_row_ready     = 2, /* Flashing Blue  */
    inst_led_row_fail      = 3, /* Flashing Red   */
    inst_led_row_success   = 4  /* Solid/Flash Green */
} inst_led_state;

/* Add capability bit for hardware with programmable LEDs */
#define INST_CAP_LED_STATUS   0x00800000

/* Inside struct _inst: */
struct _inst {
    /* ... existing methods ... */

    /* Set device indicator LED status (if supported) */
    inst_code (*set_led_state)(struct _inst *p, inst_led_state state);

    /* ... remaining struct fields ... */
};

Default instruments initialize set_led_state to NULL in inst.c.

5.2 Hardware Control Implementation: spectro/i1pro.c

5.2.1 USB Command Format

The i1Pro 2 (Rev E) microcontroller interprets LED commands sent via Bulk OUT transfers to Endpoint 0x05. Commands comprise 5-byte sequences:

cmd[0] = 0x08;        /* Subsystem Opcode: Indicator LEDs */
cmd[1] = target_led;  /* 0x00 = Both, 0x01 = Left, 0x02 = Right */
cmd[2] = r_val;       /* Red Channel (0x00 - 0xFF) or Mode Byte */
cmd[3] = g_val;       /* Green Channel (0x00 - 0xFF) */
cmd[4] = b_val;       /* Blue Channel (0x00 - 0xFF) */

Note on Rev E Firmware Revisions:
Depending on the internal microcontroller ROM revision, Byte 1 either specifies raw RGB channel levels directly or acts as an operating mode (0x00 = Off, 0x01 = Static, 0x02 = Flash/Pulse). The implementation must support a low-level static call i1pro2_raw_led(p, r, g, b) and execute timing/flashing through an internal worker thread to guarantee timing consistency across firmware revisions.

5.2.2 Flashing Management Thread

To prevent blocking synchronous measurements during strip readings, LED pulsing must execute asynchronously:

/* Thread state definition inside spectro/i1pro_imp.h */
struct _i1proimp {
    /* ... existing fields ... */
    
    /* LED thread management */
    athread        *led_th;
    int             led_th_run;
    amutex         *led_lock;
    inst_led_state  current_led_state;
};

5.2.3 Low-Level C Implementations in spectro/i1pro.c

/* Raw USB transmission helper */
static inst_code i1pro2_send_led_packet(i1proimp *p, unsigned char r, unsigned char g, unsigned char b) {
    unsigned char cmd[5];
    int trans = 0;

    if (p->devtype != inst_i1pro2)
        return inst_ok;

    if (p->icom == NULL || p->icom->is_open == NULL || !p->icom->is_open(p->icom) || p->icom->usb_rw == NULL)
        return inst_port_closed;

    memset(cmd, 0, sizeof(cmd));
    cmd[0] = 0x08;
    cmd[1] = 0x00; /* Both LEDs */
    cmd[2] = r;
    cmd[3] = g;
    cmd[4] = b;

    if (p->icom->usb_rw(p->icom, 0x05, cmd, 5, &trans, 100) != 0)
        return inst_coms_fail;

    return inst_ok;
}

/* Background flashing thread worker */
static void i1pro2_led_thread(void *context) {
    i1proimp *p = (i1proimp *)context;
    int phase = 0;

    while (p->led_th_run) {
        inst_led_state state;
        
        p->led_lock->lock(p->led_lock);
        state = p->current_led_state;
        p->led_lock->unlock(p->led_lock);

        switch (state) {
            case inst_led_cal_wait:
                /* White flashing: 500ms ON / 500ms OFF */
                if (phase % 2 == 0)
                    i1pro2_send_led_packet(p, 0xFF, 0xFF, 0xFF);
                else
                    i1pro2_send_led_packet(p, 0x00, 0x00, 0x00);
                msec_sleep(500);
                phase++;
                break;

            case inst_led_row_ready:
                /* Blue flashing: 300ms ON / 700ms OFF */
                i1pro2_send_led_packet(p, 0x00, 0x00, 0xFF);
                msec_sleep(300);
                i1pro2_send_led_packet(p, 0x00, 0x00, 0x00);
                msec_sleep(700);
                break;

            case inst_led_row_fail:
                /* Red rapid strobe: 3 short bursts */
                for (int i = 0; i < 3 && p->led_th_run; i++) {
                    i1pro2_send_led_packet(p, 0xFF, 0x00, 0x00);
                    msec_sleep(100);
                    i1pro2_send_led_packet(p, 0x00, 0x00, 0x00);
                    msec_sleep(100);
                }
                /* Lock and revert to idle/off until next command */
                p->led_lock->lock(p->led_lock);
                if (p->current_led_state == inst_led_row_fail)
                    p->current_led_state = inst_led_off;
                p->led_lock->unlock(p->led_lock);
                break;

            case inst_led_row_success:
                /* Green pulse: Solid for 400ms */
                i1pro2_send_led_packet(p, 0x00, 0xFF, 0x00);
                msec_sleep(400);
                i1pro2_send_led_packet(p, 0x00, 0x00, 0x00);
                
                p->led_lock->lock(p->led_lock);
                if (p->current_led_state == inst_led_row_success)
                    p->current_led_state = inst_led_off;
                p->led_lock->unlock(p->led_lock);
                break;

            case inst_led_off:
            default:
                i1pro2_send_led_packet(p, 0x00, 0x00, 0x00);
                msec_sleep(200);
                phase = 0;
                break;
        }
    }

    /* Extinguish LEDs upon thread exit */
    i1pro2_send_led_packet(p, 0x00, 0x00, 0x00);
}

/* Public implementation exposed via struct inst */
static inst_code i1pro_set_led_state(inst *p_inst, inst_led_state state) {
    i1proimp *p = (i1proimp *)p_inst->imp;

    if (p->devtype != inst_i1pro2)
        return inst_unsupported;

    if (p->led_lock == NULL)
        return inst_internal_error;

    p->led_lock->lock(p->led_lock);
    p->current_led_state = state;
    p->led_lock->unlock(p->led_lock);

    return inst_ok;
}

5.2.4 Initialisation and Teardown Safety

To prevent crashes on device shutdown (such as the null-pointer dereference previously observed in del_i1proimp):

In i1pro_init_inst():
Initialise mutex and thread handles if devtype == inst_i1pro2.

if (p->devtype == inst_i1pro2) {
    p->p_inst->capabilities |= INST_CAP_LED_STATUS;
    p->p_inst->set_led_state = i1pro_set_led_state;
    p->led_lock = new_amutex();
    p->led_th_run = 1;
    p->led_th = new_athread(i1pro2_led_thread, (void *)p);
}

In i1pro_close_port() and del_i1proimp():
Stop the LED thread and join it before deallocating the underlying icoms port pointers:

if (p->led_th != NULL) {
    p->led_th_run = 0;
    p->led_th->wait(p->led_th);
    p->led_th->del(p->led_th);
    p->led_th = NULL;
}
if (p->led_lock != NULL) {
    p->led_lock->del(p->led_lock);
    p->led_lock = NULL;
}
/* Defensive call while port is guaranteed open */
i1pro2_send_led_packet(p, 0x00, 0x00, 0x00);

6. Application Layer Integration (spectro/chartread.c)

6.1 CLI Flag Parsing

In spectro/chartread.c, introduce the global configuration variable:

static int g_use_leds = 0; /* Default: Do not alter LED states */

Add -L into the argv parser loop:

/* Inside main() option processing loop */
else if (argv[fa][1] == 'L') {
    g_use_leds = 1;
}

Update usage text:

fprintf(stderr, " -L                  Enable i1Pro 2 visual LED status feedback\n");

6.2 Helper Dispatch Function

Add a safe execution wrapper in chartread.c:

static void update_led_state(inst *it, inst_led_state state) {
    if (!g_use_leds || it == NULL)
        return;

    /* Verify device capability and function pointer */
    if ((it->capabilities & INST_CAP_LED_STATUS) && it->set_led_state != NULL) {
        it->set_led_state(it, state);
    }
}

6.3 Lifecycle Hook Points

Hook 1: Calibration Prompt

Locate the white tile calibration sequence in chartread.c:

/* Prior to prompting the user for calibration */
update_led_state(it, inst_led_cal_wait);

/* Existing calibration execution */
ev = it->calibrate(it, ...);

/* Turn off / reset once calibration completes */
update_led_state(it, inst_led_off);

Hook 2: Ready to Read Patch Row

At the beginning of each row cycle (strip read preparation):

/* Prompting user to read strip 'A', 'B', etc. */
printf("Hit key or button to read strip %s, or [Esc] to exit\n", row_name);
update_led_state(it, inst_led_row_ready);

/* Enter strip reading execution */
rv = it->read_strip(it, ...);

Hook 3: Scan Evaluation (Success vs Failure)

Inspect the return code of read_strip():

if (rv == inst_ok) {
    /* Successful pass */
    update_led_state(it, inst_led_row_success);
    printf("Strip %s successfully read.\n", row_name);
} else {
    /* Misread, speed error, or optical alignment failure */
    update_led_state(it, inst_led_row_fail);
    printf("Failed to read strip %s - please repeat.\n", row_name);
}

Hook 4: Cleanup on Exit

Ensure all LEDs are extinguished when chartread exits cleanly or via error branches:

update_led_state(it, inst_led_off);

7. Verification & Testing Matrix

Execute the following test cases to validate conformance:

ID Test Scenario Hardware Setup Command Invocation Expected Behaviour
TC-01 Standard Legacy Baseline i1Pro 2 (Rev E) chartread target Default operation; LEDs remain completely extinguished throughout.
TC-02 Unsupported Hardware Fallback i1Pro Rev D / ColorMunki chartread -L target -L switch parsed; driver detects lack of INST_CAP_LED_STATUS; runs normally with zero errors.
TC-03 White Calibration Interlock i1Pro 2 (Rev E) chartread -L target Ring flashes White until calibration completes; turns off upon leaving the tile.
TC-04 Row Read Readiness i1Pro 2 (Rev E) chartread -L target Ring flashes Blue while waiting for strip swipe initiation.
TC-05 Strip Read Failure Detection i1Pro 2 (Rev E) chartread -L target Intentional misread (erratic speed); ring flashes Red rapidly for ~600 ms, then returns to Blue ready state.
TC-06 Strip Read Success i1Pro 2 (Rev E) chartread -L target Valid swipe; ring illuminates solid Green for 400 ms, then transitions to Blue for the subsequent row.
TC-07 Session Abort (SIGINT / Esc) i1Pro 2 (Rev E) chartread -L target Aborting via Ctrl+C or Esc; background worker terminates cleanly; LEDs extinguish with exit code 0.

**Target Subsystem:** `spectro/chartread.c`, `spectro/inst.h`, `spectro/i1pro.c`, `spectro/i1pro_imp.h` **Supported Hardware:** X-Rite i1Pro 2 (Rev E) **Deliverable Format:** Markdown Technical Implementation Specification --- ## 1. Executive Summary This document specifies the technical architecture and code modifications required to implement real-time visual status feedback via the integrated ring LEDs on the X-Rite i1Pro 2 spectrophotometer within ArgyllCMS's `chartread` utility. The feature introduces a dedicated command-line flag to govern LED signaling across distinct stages of the chart-reading lifecycle: | Stage | Visual Indicator | |-------|------------------| | Awaiting Base Calibration | Flashing White | | Ready to Scan Row | Flashing Blue | | Row Read Failed / Scan Error | Flashing Red | | Row Read Succeeded | Flashing Green | If the command-line flag is omitted, or if an instrument lacking programmable RGB indicators (such as an i1Pro Rev A–D, ColorMunki, or third-party colorimeter) is connected, the operational behaviour degrades silently to default ArgyllCMS mechanics without throwing errors or halting execution. --- ## 2. Command-Line Switch Selection ### 2.1 Audit of Existing `chartread` Options An audit of `spectro/chartread.c` argument parsing confirms the following active options: | Flag | Argument | Function | |------|----------|----------| | `-v` | `[level]` | Verbose execution mode | | `-c` | `port` | Communication port selection | | `-t` | None | Transmissive chart read mode | | `-d` | None | Direct display reading mode | | `-p` | None | Spot / patch-by-patch mode instead of strip mode | | `-r` | None | Resume an incomplete chart session | | `-n` | None | Do not save incomplete chart data | | `-N` | None | Suppress initial calibration prompt if possible | | `-B` | None | Suppress bi-directional strip reading | | `-H` | None | High-resolution spectral data collection | | `-F` | `filter` | Filter configuration (`none`, `pol`, `d65`, etc.) | | `-T` | `factor` | Patch recognition tolerance factor | | `-A` | `factor` | Patch recognition averaging factor | | `-y` | `type` | Display type specification | | `-X` | `file.cal` | External calibration file override | ### 2.2 Selected Switch: `-L` The uppercase letter `-L` is unused in `chartread` and serves as a mnemonic for **LED** status indicators. - **Usage Syntax:** `chartread -L [other_options] basename` - **Parsing Semantics:** Boolean flag; requires no parameter argument. - **Fallback Contract:** When `-L` is passed but the attached spectrophotometer does not expose the `inst_stat_leds` capability, the flag is silently ignored. --- ## 3. LED Functional State Matrix The i1Pro 2 (Rev E) features two multi-colour LEDs flanking the optical button. Both LEDs are driven concurrently to ensure 360° visibility. | Lifecycle State | Target Visual Pattern | Dominant Colour | Duty Cycle / Timing | Termination Trigger | |-----------------|-----------------------|-----------------|---------------------|---------------------| | Awaiting Calibration | Dual Flash / Pulse | White (`#FFFFFF`) | 500 ms ON / 500 ms OFF (1.0 Hz) | Instrument leaves calibration plate, or button/Enter is pressed. | | Ready for Row Scan | Dual Pulse | Blue (`#0000FF`) | 300 ms ON / 700 ms OFF (1.0 Hz) | Measurement button pressed, optical sensor detects movement, or swipe initiates. | | Row Read Failure | Rapid Strobe | Red (`#FF0000`) | 100 ms ON / 100 ms OFF (3 pulses, 600 ms total) | Automatically reverts to Ready for Row Scan (Flashing Blue) upon re-prompting. | | Row Read Success | Confirmation Flash | Green (`#00FF00`) | 400 ms solid illumination | Automatically extinguishes, then transitions to Ready for Row Scan for next row. | | Shutdown / Standby | Extinguished | Off (`#000000`) | Static 0% duty cycle | Invoked upon port closure, session completion, or `SIGINT`. | --- ## 4. Architectural Design & Abstraction Layers To maintain clean separation between Argyll's hardware-agnostic application layer (`chartread.c`) and device-specific driver code, the implementation is divided across two architectural boundaries: ``` +-------------------------------------------------------------+ | chartread.c | | - Parses '-L' switch into 'g_use_leds' global flag | | - Calls inst->set_led_state() at state transition points | +-------------------------------------------------------------+ | | (inst_led_state enum) v +-------------------------------------------------------------+ | inst.h / inst.c | | - Exposes inst_led_state typedef | | - Defines INST_CAP_LED_STATUS capability bitmask | | - Declares inst_code (*set_led_state)(inst *p, ...) | +-------------------------------------------------------------+ | | (Internal Driver Dispatch) v +-------------------------------------------------------------+ | spectro/i1pro.c | | - Implements i1pro2_set_led_state() | | - Manages background LED flashing thread or firmware modes | | - Writes 5-byte command packets to USB Endpoint 0x05 | +-------------------------------------------------------------+ ``` --- ## 5. Driver Layer Implementation (`spectro/`) ### 5.1 Interface Extensions: `spectro/inst.h` Add enumeration states and function pointer signatures to the generic instrument interface: ```c /* --- spectro/inst.h additions --- */ /* Generic Instrument LED Status States */ typedef enum { inst_led_off = 0, inst_led_cal_wait = 1, /* Flashing White */ inst_led_row_ready = 2, /* Flashing Blue */ inst_led_row_fail = 3, /* Flashing Red */ inst_led_row_success = 4 /* Solid/Flash Green */ } inst_led_state; /* Add capability bit for hardware with programmable LEDs */ #define INST_CAP_LED_STATUS 0x00800000 /* Inside struct _inst: */ struct _inst { /* ... existing methods ... */ /* Set device indicator LED status (if supported) */ inst_code (*set_led_state)(struct _inst *p, inst_led_state state); /* ... remaining struct fields ... */ }; ``` Default instruments initialize `set_led_state` to `NULL` in `inst.c`. ### 5.2 Hardware Control Implementation: `spectro/i1pro.c` #### 5.2.1 USB Command Format The i1Pro 2 (Rev E) microcontroller interprets LED commands sent via Bulk OUT transfers to Endpoint `0x05`. Commands comprise 5-byte sequences: ```c cmd[0] = 0x08; /* Subsystem Opcode: Indicator LEDs */ cmd[1] = target_led; /* 0x00 = Both, 0x01 = Left, 0x02 = Right */ cmd[2] = r_val; /* Red Channel (0x00 - 0xFF) or Mode Byte */ cmd[3] = g_val; /* Green Channel (0x00 - 0xFF) */ cmd[4] = b_val; /* Blue Channel (0x00 - 0xFF) */ ``` **Note on Rev E Firmware Revisions:** Depending on the internal microcontroller ROM revision, Byte 1 either specifies raw RGB channel levels directly or acts as an operating mode (`0x00` = Off, `0x01` = Static, `0x02` = Flash/Pulse). The implementation must support a low-level static call `i1pro2_raw_led(p, r, g, b)` and execute timing/flashing through an internal worker thread to guarantee timing consistency across firmware revisions. #### 5.2.2 Flashing Management Thread To prevent blocking synchronous measurements during strip readings, LED pulsing must execute asynchronously: ```c /* Thread state definition inside spectro/i1pro_imp.h */ struct _i1proimp { /* ... existing fields ... */ /* LED thread management */ athread *led_th; int led_th_run; amutex *led_lock; inst_led_state current_led_state; }; ``` #### 5.2.3 Low-Level C Implementations in `spectro/i1pro.c` ```c /* Raw USB transmission helper */ static inst_code i1pro2_send_led_packet(i1proimp *p, unsigned char r, unsigned char g, unsigned char b) { unsigned char cmd[5]; int trans = 0; if (p->devtype != inst_i1pro2) return inst_ok; if (p->icom == NULL || p->icom->is_open == NULL || !p->icom->is_open(p->icom) || p->icom->usb_rw == NULL) return inst_port_closed; memset(cmd, 0, sizeof(cmd)); cmd[0] = 0x08; cmd[1] = 0x00; /* Both LEDs */ cmd[2] = r; cmd[3] = g; cmd[4] = b; if (p->icom->usb_rw(p->icom, 0x05, cmd, 5, &trans, 100) != 0) return inst_coms_fail; return inst_ok; } /* Background flashing thread worker */ static void i1pro2_led_thread(void *context) { i1proimp *p = (i1proimp *)context; int phase = 0; while (p->led_th_run) { inst_led_state state; p->led_lock->lock(p->led_lock); state = p->current_led_state; p->led_lock->unlock(p->led_lock); switch (state) { case inst_led_cal_wait: /* White flashing: 500ms ON / 500ms OFF */ if (phase % 2 == 0) i1pro2_send_led_packet(p, 0xFF, 0xFF, 0xFF); else i1pro2_send_led_packet(p, 0x00, 0x00, 0x00); msec_sleep(500); phase++; break; case inst_led_row_ready: /* Blue flashing: 300ms ON / 700ms OFF */ i1pro2_send_led_packet(p, 0x00, 0x00, 0xFF); msec_sleep(300); i1pro2_send_led_packet(p, 0x00, 0x00, 0x00); msec_sleep(700); break; case inst_led_row_fail: /* Red rapid strobe: 3 short bursts */ for (int i = 0; i < 3 && p->led_th_run; i++) { i1pro2_send_led_packet(p, 0xFF, 0x00, 0x00); msec_sleep(100); i1pro2_send_led_packet(p, 0x00, 0x00, 0x00); msec_sleep(100); } /* Lock and revert to idle/off until next command */ p->led_lock->lock(p->led_lock); if (p->current_led_state == inst_led_row_fail) p->current_led_state = inst_led_off; p->led_lock->unlock(p->led_lock); break; case inst_led_row_success: /* Green pulse: Solid for 400ms */ i1pro2_send_led_packet(p, 0x00, 0xFF, 0x00); msec_sleep(400); i1pro2_send_led_packet(p, 0x00, 0x00, 0x00); p->led_lock->lock(p->led_lock); if (p->current_led_state == inst_led_row_success) p->current_led_state = inst_led_off; p->led_lock->unlock(p->led_lock); break; case inst_led_off: default: i1pro2_send_led_packet(p, 0x00, 0x00, 0x00); msec_sleep(200); phase = 0; break; } } /* Extinguish LEDs upon thread exit */ i1pro2_send_led_packet(p, 0x00, 0x00, 0x00); } /* Public implementation exposed via struct inst */ static inst_code i1pro_set_led_state(inst *p_inst, inst_led_state state) { i1proimp *p = (i1proimp *)p_inst->imp; if (p->devtype != inst_i1pro2) return inst_unsupported; if (p->led_lock == NULL) return inst_internal_error; p->led_lock->lock(p->led_lock); p->current_led_state = state; p->led_lock->unlock(p->led_lock); return inst_ok; } ``` #### 5.2.4 Initialisation and Teardown Safety To prevent crashes on device shutdown (such as the null-pointer dereference previously observed in `del_i1proimp`): **In `i1pro_init_inst()`:** Initialise mutex and thread handles if `devtype == inst_i1pro2`. ```c if (p->devtype == inst_i1pro2) { p->p_inst->capabilities |= INST_CAP_LED_STATUS; p->p_inst->set_led_state = i1pro_set_led_state; p->led_lock = new_amutex(); p->led_th_run = 1; p->led_th = new_athread(i1pro2_led_thread, (void *)p); } ``` **In `i1pro_close_port()` and `del_i1proimp()`:** Stop the LED thread and join it before deallocating the underlying `icoms` port pointers: ```c if (p->led_th != NULL) { p->led_th_run = 0; p->led_th->wait(p->led_th); p->led_th->del(p->led_th); p->led_th = NULL; } if (p->led_lock != NULL) { p->led_lock->del(p->led_lock); p->led_lock = NULL; } /* Defensive call while port is guaranteed open */ i1pro2_send_led_packet(p, 0x00, 0x00, 0x00); ``` --- ## 6. Application Layer Integration (`spectro/chartread.c`) ### 6.1 CLI Flag Parsing In `spectro/chartread.c`, introduce the global configuration variable: ```c static int g_use_leds = 0; /* Default: Do not alter LED states */ ``` Add `-L` into the `argv` parser loop: ```c /* Inside main() option processing loop */ else if (argv[fa][1] == 'L') { g_use_leds = 1; } ``` Update usage text: ```c fprintf(stderr, " -L Enable i1Pro 2 visual LED status feedback\n"); ``` ### 6.2 Helper Dispatch Function Add a safe execution wrapper in `chartread.c`: ```c static void update_led_state(inst *it, inst_led_state state) { if (!g_use_leds || it == NULL) return; /* Verify device capability and function pointer */ if ((it->capabilities & INST_CAP_LED_STATUS) && it->set_led_state != NULL) { it->set_led_state(it, state); } } ``` ### 6.3 Lifecycle Hook Points **Hook 1: Calibration Prompt** Locate the white tile calibration sequence in `chartread.c`: ```c /* Prior to prompting the user for calibration */ update_led_state(it, inst_led_cal_wait); /* Existing calibration execution */ ev = it->calibrate(it, ...); /* Turn off / reset once calibration completes */ update_led_state(it, inst_led_off); ``` **Hook 2: Ready to Read Patch Row** At the beginning of each row cycle (strip read preparation): ```c /* Prompting user to read strip 'A', 'B', etc. */ printf("Hit key or button to read strip %s, or [Esc] to exit\n", row_name); update_led_state(it, inst_led_row_ready); /* Enter strip reading execution */ rv = it->read_strip(it, ...); ``` **Hook 3: Scan Evaluation (Success vs Failure)** Inspect the return code of `read_strip()`: ```c if (rv == inst_ok) { /* Successful pass */ update_led_state(it, inst_led_row_success); printf("Strip %s successfully read.\n", row_name); } else { /* Misread, speed error, or optical alignment failure */ update_led_state(it, inst_led_row_fail); printf("Failed to read strip %s - please repeat.\n", row_name); } ``` **Hook 4: Cleanup on Exit** Ensure all LEDs are extinguished when `chartread` exits cleanly or via error branches: ```c update_led_state(it, inst_led_off); ``` --- ## 7. Verification & Testing Matrix Execute the following test cases to validate conformance: | ID | Test Scenario | Hardware Setup | Command Invocation | Expected Behaviour | |----|---------------|----------------|--------------------|--------------------| | TC-01 | Standard Legacy Baseline | i1Pro 2 (Rev E) | `chartread target` | Default operation; LEDs remain completely extinguished throughout. | | TC-02 | Unsupported Hardware Fallback | i1Pro Rev D / ColorMunki | `chartread -L target` | `-L` switch parsed; driver detects lack of `INST_CAP_LED_STATUS`; runs normally with zero errors. | | TC-03 | White Calibration Interlock | i1Pro 2 (Rev E) | `chartread -L target` | Ring flashes White until calibration completes; turns off upon leaving the tile. | | TC-04 | Row Read Readiness | i1Pro 2 (Rev E) | `chartread -L target` | Ring flashes Blue while waiting for strip swipe initiation. | | TC-05 | Strip Read Failure Detection | i1Pro 2 (Rev E) | `chartread -L target` | Intentional misread (erratic speed); ring flashes Red rapidly for ~600 ms, then returns to Blue ready state. | | TC-06 | Strip Read Success | i1Pro 2 (Rev E) | `chartread -L target` | Valid swipe; ring illuminates solid Green for 400 ms, then transitions to Blue for the subsequent row. | | TC-07 | Session Abort (`SIGINT` / Esc) | i1Pro 2 (Rev E) | `chartread -L target` | Aborting via Ctrl+C or Esc; background worker terminates cleanly; LEDs extinguish with exit code 0. | ---
gronod added the Kind/Feature
Reviewed
Confirmed
1
Priority
Medium
3
labels 2026-09-04 16:52:15 +01:00
Sign in to join this conversation.
1 Participants
Notifications
Due Date
No due date set.
Reference: gronod/argyllcms#37