this proves skipped commands are likely due to the fact that the buffer is overwritten while process_ii is executing. the solution can either be the above or having process_ii make a copy of the buffer data before it starts processing.
still getting freezing with CV 6 CV 5 in M script…
one more thing in the code above - added a check for rx_pos not to go over I2C_RX_BUF_SIZE
good find, i was thinking this but couldn’t rationalize it being an issue-- so what this suggests is the I2C interrupt (for rx) is jumping in and plowing the data before the previous interrupt’s process function has finished, augh.
i’ll stress-test with this fix and then jump into the metro issues once it feels solid (i’m keeping the metro issue separate for now)
It’s normal for a master receiver to indicate the end of reception by sending a NACK. It needs to do this so it can take control of the bus and issue the STOP condition. If it sent an ACK instead, the state machine of the transmitter slave may attempt to continue to drive the bus.
at this point pretty sure the reason something like CV 6 CV 5 locks in Metro script is because it executes op_CV_get which writes to and reads from i2c immediately, then it executes op_CV_set which queues an i2c event. so it’s possible that the next op_CV_get (triggered by the clock interrupt handler) tries to execute while there is a transmission in progress initiated by the i2c event handler, so it ends up corrupting it and leaving TT hanging.
this is confirmed by the fact that if i add if (i2c_waiting_count) return; to tele_ii_rx it doesn’t freeze anymore (but it never seems to read either).
with the current architecture i can’t think of a way to queue reads since it can’t proceed until a read is completed, so the only option seems to make any i2c communications atomic. so i think @ngwese’s solution to disable interrupts while i2c transmission is in progress is probably the only way to handle it. either that or any interrupts should just create events and the actual execution should happen within check_events.
edit: another option is to cache CV values, have op_CV_get return cached values, queue reads as i2c events and have the event handler update the cache. this means that the values are not guaranteed to be up to date - more of an issue for TXi though, since for ansible TT can update its cache when it writes to ansible (unless CVs can change outside of TT - like maybe reading CV values produced by, say, Cycles?). either way, it’ll likely be a tug war between how accurate you want CV reads to be vs the timing accuracy of TT.
for the rx_buffer fix do you want me to do a pull request? that was a quick PoC code, could also be tightened up a bit - number of read buffers should be equivalent to MAX_EVENTS, and perhaps each buffer should include a flag indicating whether it was read by process_ii already which could be used by twi_slave_rx to find the next free buffer to write to.
Without knowing too much about the specifics of the Teletype code I would suggest that the I2C interrupts should get masked once the reception is complete (I guess that’s in twi_slave_stop ?). Then the contents of the buffer should be copied to a command queue which the main code checks for new commands. That’s how I’ve done it in the Intellijel modules, not just with I2C but any interrupt-driven comms.
If the copy of the buffer is fast enough you might not even need to mask interrupts, though it doesn’t hurt…
didn’t make too many changes - i was confused about using MAX_EVENTS since process_ii doesn’t use the event queue. 8 buffers seems enough, i tested with a 10ms Metro. also just incrementing the index is the best/easiest way to find a free buffer anyway, so don’t need a separate flag.
Regarding this, and @scanner_darkly’s earlier mention about the address seeming strange: On the JF I had to left-shift the address we created in TT by 1bit in order for them to match up in terms of i2c. Not sure whether it’s avr or stm who messed up the 7bit address thing. Apparently stm packs those 7bits into the 7MSBs vs avr’s 7LSBs.
It also freezes in Cycles when I patch a cable from input 2 to Meadowphysics. Now after freezing a few times the CV output of cycles is heavily stepped again while it has been continuous before (after doing the new update).