Merge pull request #12 from dihm/adm_error_handling

Adm error handling

Author: dihm

README.md

@@ -59,8 +59,10 @@ These commands must be run when the running status is `STOPPED`.
 * `get <address (in hex)>` - Gets instruction at address. Returns output word and number of clock cycles separated by a space, in same format as `set`.
 * `run` - Used to hardware start a programmed sequence (ie waits for external trigger before processing first instruction).
 * `swr` - Used to software start a programmed sequence (ie do not wait for a hardware trigger at sequence start).
-* `adm <number of instructions (in hex)>` - Enters mode for adding pulse instructions in binary.
-  * The number of instructions must be specified with the command. The Pi Pico will then wait for that enough bytes to fill that many instructions (6 times the number of instructions) to be read, and will not respond during this time unless there is an error. This mode can not be be terminated until that many bytes are read.
+* `adm <starting instruction address (in hex)> <number of instructions (in hex)>` - Enters mode for adding pulse instructions in binary.
+  * This command over-writes any existing instructions in memory. The starting instruction address specifies where to insert the block of instructions. This is generally set to 0 to write a complete instruction set from scratch.
+  * The number of instructions must be specified with the command, which is used to determine the total number of bytes to be read (6 6 times the number of instructions).
+  * This command returns `ready\r\n` to signify it is ready for binary data. The Pico will then read the total number of bytes. This mode can not be terminated until that many bytes are read.
   * Each instruction is specified by a 16 bit unsigned integer (little Endian, output 15 is most significant) specifying the state of the outputs and a 32 bit unsigned integer (little Endian) specifying the number of clock cycles.
     * The number of clock cycles sets how long this state is held before the next instruction.
     * If the number of clock cycles is 0, this indicates an indefinite wait.
@@ -85,27 +87,50 @@ The basis of the functionality for this serial interface was developed by Carter
 ## Clock Sync
 Firmware supports the use of an external clock. This prevents any significant phase slip between a pseudoclock and this digital output controller if their clocks are phase synchronous. Without external buffering hardware, clock must be LVCMOS compatible.
 
-## Example:
-Below program sets one output high for 1 microsecond, then low while setting the next output high for 1 microsecond (64 in hex = 100 in decimal) for 6 outputs, then stopping:
+## Examples:
+Below python script sets one output high for 1 microsecond, then low while setting the next output high for 1 microsecond (64 in hex = 100 in decimal) for 6 outputs, then stopping. `do` is a pyserial handle to the pico.
 
-Commands (`\n` explicitly denotes newline/pressing enter):
+```python
+bits = [1, 2, 3, 8, 10, 20, 0, 0]
+cycles = [100, 100, 100, 100, 100, 100, 0, 0]
 
-```
-add\n
-1 64\n
-2 64\n
-4 64\n
-8 64\n
-10 64\n
-20 64\n
-0 0\n
-0 0\n
-end\n
-```
+do.write('add\n'.encode())
+for bit, cycle in zip(bits, cycles):
+  do.write(f'{bit:x} {cycle:x}\n'.encode())
 
+do.write('end\n'.encode())
+resp = do.readline().decode()
+assert resp == 'ok\r\n'
+```
 
+Output of the above sequence.
 <img width="470" alt="documentation" src="https://github.com/pmiller2022/prawn_digital_output/assets/75953337/932b784f-346f-4598-8679-b857578e0291">
 
+This is a python script that employs the binary write `adm` command.
+In order to function correctly, the data to be written must be in a numpy structured array where each column dtype can be specified uniquely.
+Below assumes the `bits` and `cycles` are 1-D arrays that have already been created.
+It also assumes `do` is a pyserial handle to the device. 
+
+```python
+data = np.zeros(len(bits), dtype=[('bits', 'u2'), ('cycles','u4')])
+data['bits'] = bits
+data['cycles'] = cycles
+
+serial_buffer = data.tobytes()
+
+do.write(f'adm 0 {len(data):x}\r\n'.encode())
+resp = do.readline().decode()
+assert resp == 'ready\r\n', f'Not ready for binary data. Response was {repr(resp)}'
+do.write(serial_buffer)
+resp = do.readline().decode()
+if resp != 'ok\r\n':
+  # if response not ok, probably more than one line, read them all
+  # done this way to prevent readlines timeout for standard operation
+  extra_resp = do.readlines()
+  resp += ''.join([st.decode() for st in extra_resp])
+  print(f'Write had errors. Response was {repr(resp)}')
+```
+
 ## Compiling the firmware
 
 If you want to make changes to the firmware, or want to compile it yourself (because you don't trust binary blobs from the internet), we provide a docker configuration to help you do that.

prawn_do/prawn_do.c

@@ -519,17 +519,28 @@ int main(){
 		// Add many command: read in a fixed number of binary integers without separation,
 		// append to command array
 		else if(strncmp(serial_buf, "adm", 3) == 0){
-			// Get how many instructions this adm command contains
+			// Get how many instructions this adm command contains and where to insert them
+			uint32_t start_addr;
 			uint32_t inst_count;
-			int parsed = sscanf(serial_buf, "%*s %x", &inst_count);
-			if(parsed < 1){
+			int parsed = sscanf(serial_buf, "%*s %x %x", &start_addr, &inst_count);
+			if(parsed < 2){
 				fast_serial_printf("Invalid request\r\n");
+				continue;
 			}
-
 			// Check that the instructions will fit in the do_cmds array
-			if(inst_count + do_cmd_count >= MAX_DO_CMDS - 3){
-				fast_serial_printf("Too many DO commands (%d + %d). Please use resources more efficiently or increase MAX_DO_CMDS and recompile.\r\n", do_cmd_count, inst_count);
+			else if(inst_count + start_addr > MAX_INSTR){
+				fast_serial_printf("Invalid address and/or too many instructions (%d + %d).\r\n", start_addr, inst_count);
+				continue;
 			}
+			else{
+				fast_serial_printf("ready\r\n");
+			}
+
+			// reset do_cmd_count to start_address
+			do_cmd_count = start_addr * 2;
+
+			uint32_t reps_error_count = 0;
+			uint32_t last_reps_error_idx = 0;
 
 			// It takes 6 bytes to describe an instruction: 2 bytes for values, 4 bytes for time
 			uint32_t inst_per_buffer = SERIAL_BUFFER_SIZE / 6;
@@ -545,7 +556,9 @@ int main(){
 									 | (serial_buf[6*i+3] << 8)
 									 | serial_buf[6*i+2]);
 					if(reps < 5 && reps != 0){
-						fast_serial_printf("Reps must be 0 or greater than 4, got %x\r\n", reps);
+						reps_error_count++;
+						last_reps_error_idx = (do_cmd_count + 1) / 2;
+						reps = 0;
 					}
 					if(reps != 0){
 						reps -= 4;
@@ -569,7 +582,9 @@ int main(){
 									 | (serial_buf[6*i+3] << 8)
 									 | serial_buf[6*i+2]);
 					if(reps < 5 && reps != 0){
-						fast_serial_printf("Reps must be 0 or greater than 4, got %x\r\n", reps);
+						reps_error_count++;
+						last_reps_error_idx = (do_cmd_count + 1) / 2;
+						reps = 0;
 					}
 					if(reps != 0){
 						reps -= 4;
@@ -579,7 +594,12 @@ int main(){
 				}
 			}
 
-			fast_serial_printf("ok\r\n");
+			if(reps_error_count > 0){
+				fast_serial_printf("Invalid number of reps in %d instructions, most recent error at instruction %d. Setting reps to zero for these instructions.\r\n", reps_error_count, last_reps_error_idx);
+			}
+			else{
+				fast_serial_printf("ok\r\n");
+			}
 		}
 		// Dump command: print the currently loaded buffered outputs
 		else if(strncmp(serial_buf, "dmp", 3) == 0){