monome / crow

Crow speaks and listens and remembers bits of text. A scriptable USB-CV-II machine
GNU General Public License v3.0
166 stars 34 forks source link

Communication layer debugging #98

Closed trentgill closed 5 years ago

trentgill commented 5 years ago

Fundamentally there's 3 categories of issues we're seeing in the communication layer:

  1. The usb driver itself crashing with high throughput.
  2. crow's lua env crashes when sending data at high speeds to USB.
  3. Long (or all?) messages to crow are not 100% reliable.

usb driver crash

crow env crash

Streaming both CV inputs to Max using the input[n].mode('stream', time) functions where time is set to 0.001 for 1000samples/s. When this is the only thing happening, transfer is reliable (no issue >10minutes).

As soon as the standard ASL LFOs are running, crashes always happen at varying rates.

Note, the input stream functions by the IRQ event calling into lua and running the function assigned in the input[n] table under stream. The default callback is used which sends a string ^^ret_cv(1,value) over usb via the _c.tell() function.

Inferences & suggestions: Fundamentally it appears the lua crashes are the result of interrupt conflicts. potentially due to one callback being inside a lua process when another interrupt calls into that same lua process. The clear solution to this problem is implementing an event queue for lua which is processed in the main loop (as norns does). This way we can ensure that we don't have simultaneous calls into lua. @tehn we should have a discussion about how to implement such an event queue. I'd like to keep it as minimal as possible for now and not worry about prioritization or other nice-to-have features.

Additionally, it would be possible to reduce the number of calls into lua altogether by implementing the default callbacks in C, and setting a switch in lua. I don't like this as much as it somewhat complicates the actions and muddies the 'crow is just a lua env w hardware hooks' mentality.

To help reduce memory usage & time-in-lua, the _c.tell() function (which creates the formatted messages for sending luachunks to a host device) should be a C function. Implementation is simple and it only sacrifices a small amount of customizability (the function names need to be defined in C). A lua version can obviously remain too, allowing for full customization with the increased overhead of string handling in lua. This may also help with string integrity crow->host.

data reliability

tehn commented 5 years ago

i'm glad to see you're having similar thoughts. after my time with crow yesterday and this morning these are my recommendations (basically repeating what you wrote):

the input module i don't immediately see any trouble moving to c with a loss of existing features... though i have to more deeply at the various use cases.

the output module (ASL) however, does not seem like it could exist as designed. right now the fundamental feature (chaining functions together, which is super rad) relies on callbacks into lua, which i am starting to feel should be avoided. i'm suspecting that ASL might not be a good fit for this platform... but we can see how it behaves after an event system is in place. an alternative would be more narrow set of features: set target, slew, shape. maybe implement a "segment" system with a looping mode, so envelopes could be build by sending multiple segments.

i can start working on the event system on sunday.

trentgill commented 5 years ago

Migrating input module to C The only things that happens in lua are

  1. setting up the configuration
  2. handling timed / event-detection callbacks

The setup should pose no problem to occur inside of lua. If we migrate the callback to C then I don't understand how it's customizable from lua? That would be fine for the crow-remote (and we can make this default action a C-only event), but seems it would compromise functionality in a standalone case. I think we should wait on this decision after building the event system.

Regarding ASL it does indeed need to callback into lua at each breakpoint. It would be possible to create 'pre-compiled' ASL structures that exist solely in C (this is the idea for handling audio-rate ASLs), however it compromises the ability to query real-time values (eg. input state, random number gen, dynamic sequencer info). Again I think we should wait to see how the event system changes the behaviour.

I'm moving the tell function to C right now, then I'll spend some more time on the usb_rx crash. Happy to take a look at the event system later as I have today blocked out for crow. Are the teletype or norns event queues particularly relevant?

tehn commented 5 years ago

thanks for the details re: migration. will think it over further.

norns event system is a perfect foundation to emulate.

tehn commented 5 years ago

this is a side issue, but when USB isn't connected there is an endless stream of

CDC_tx failed 2

printed via UART which certainly will bog standalone mode.

perhaps there's a different way to detect USB presence before attempting CDC_tx?

trentgill commented 5 years ago

Yes totally, I'll make a separate issue for that.

trentgill commented 5 years ago

Remaining issues:

trentgill commented 5 years ago

closing this as per #116