bees serial protocol

[ghost]

Over on my dev branch there is a bees serial protocol enabling the following operations from host.

remote control of network:

• read/write DSP params from bees (tested, working)
• read/write bees inputs (tested, working)
• connect bees out to bees in (tested, dummy-only)
• disconnect bees out (tested, dummy-only)
• create op (tested, dummy-only)
• delete op (tested, dummy-only)
• save preset (tested, dummy-only)
• recall preset (tested, dummy-only)

bees network discovery:

• dump ins in netlist (tested, working)
• dump params in netlist (tested, working)
• dump outs in netlist (tested, dummy-only)
• dump ops in netlist (tested, dummy-only)
• dump connections in netlist (tested, dummy-only)
• dump op descriptions (tested, dummy-only)

callback to host:

• special bees operator sends s16 addr, s16 data (tested, working)

Planning to send pull request once all the untested items are known-working. Patching commands & presets will be dummy-only for now but think plumbing these in is straightforward. Host side is currently implemented as a common lisp serial test harness. Docs to follow once the protocol is proven in my host application (lisp midilooper).

callback to host functionality as it stands (op_serial) still kind of sucks... Eventually the protocol should be updated replacing bees serial op with these message types:

• toggle serial message on output & associated callback (requires a tricky refactor of bees)
• toggle serial message on DSP param callback (DSP callback does not currently exist)
• toggle serial message on input & associated callback (should be easy to add)

[catfact]

DSP callback: #244

[ghost]

Hmmmph - once I get more familiar with OSC for bfin simulator (see #245) I may start to regret rolling my own serial protocol rather than try and hook into an OSC-over-serial arduino lib or similar. What does the wire protocol for monome grids look like?

Any thoughts on this? Performance vs ease-of-code vs fitness-for-purpose tradeoffs?

[tehn]

monome stuff is serial via FTDI, converted to OSC with serialosc (our own
daemon thing). http://monome.org/docs/serial.txt

i suspect direct-serial is a good path for aleph communication.

apologies for the line silence on my end-- insane cramming before we head
out for a bit-- whereupon then i'll have mental time to process everything
you guys have been up to. looks incredible.

On Tue, Feb 2, 2016 at 3:09 PM, rick notifications@github.com wrote:

Hmmmph - once I get more familiar with OSC for bfin simulator (see #245
#245) I may start to regret rolling
my own serial protocol rather than try and hook into an OSC-over-serial
arduino lib or similar. What does the wire protocol for monome grids look
like?

Any thoughts on this? Performance vs ease-of-code vs fitness-for-purpose
tradeoffs?

—
Reply to this email directly or view it on GitHub
#243 (comment).

[catfact]

i don't love the idea of shoving OSC parsing into bees, seems kinda heavy and/or lots of work to strip it down to something appropriately tiny.

easier just to make a host-side bridge program using liblo (or python, like serialosc.) liblo is very, very easy to use.

[ngwese]

i haven't looked at the serial protocol which has been crafted but one thought crossed my mind... if the protocol is simple enough it could be interesting to use some (or all) of the serial machinery over the ii bus (aka i2c). i do occasionally think about what having a teletype capable of sending values into a bees network might look like...

[ghost]

since there seems to be a consensus that what we already have is probably the right thing, I will try to keep moving forward with that.

First real-world test for the protocol will be to integrate the existing aleph communication into my prototype midilooper host application.

Once a few melodies have been tapped out on quneo pads playing grains like a sampler time to address #244 & #251. Then it will be possible to replace op_param with the full serial functionality toggling serial messages on outs/ins/params/param-callbacks.

[ghost]

ha - so just got a few neat little bits and bobs on the go using lisp code like randomising pitchshift arpeggiator & other such silliness. Nothing that can't be done from bees already but kind of exciting anyway.

The bad news is the new serial coms does seem to overload bees very easily especially if you start zooming around the menu with encoders or buttons whilst sending as little as 20 messages per second. No idea why this might be at present. Going to try flashing non-debug bees maybe everything stops blowing up...

EDIT: Just posted some total nonsense & deleted the post. Somewhat stuck on this & flailing around a bit sorry for the spam...

[ghost]

Set up a debug variable to print number of serial events since last complete message, and the chars in that message were going through the event loop one at a time (though reading serial.c properly this doesn't seem to be the intention).

Just tried adding a hack to serial.c to only trigger events after stop char received (just pushed that on my dev branch, though it may need to be reverted). It passes serial messages in one go but does not fix the problem.

still unstable if you try to use encoders in menus whilst hitting aleph with even 10 serial commands a second. Blows up somewhere above 20 messages per second in play mode not touching the controls. This behaviour also highly reproducible. 20 messages per second will run for over 5 minutes, whereas 25 messages per second nearly instant death. So I guess all the interrupt logic, scheduling etc is sound or things would be less predictable.

This behaviour seems to me consistent with running out of cycles. Maybe just inefficient serial protocol code that is to blame. test message is only 7 bytes - same behaviour whether triggering bees in or bfin param. I'm totally stumped - any suggestions dudes?

EDIT: was confusing messages per sec with ms per message posting late last night (corrected). Also realised the obvious point after sleep that printing param/bees-in to screen in play mode is stress-testing the drawing code as well as the serial.

[catfact]

can you link to the code where you're actually calling serial_send_ byte() or serial_send_long() or whatever? i can't quite find it right now.

it's true that drawing is kind of heavy. drawing to a screen region (aka rect) doesn't trigger a screen refresh directly; pixels are set and a dirty flag is set for the region. in bees, dirty regions are updated on a 50ms timer.

but, rendering with fonts is kind of heavy in itself; each of those calls to font_string_region_clip() has to loop over each glyph in the string, each column in the glyph...

as for the serial update itself, looks like serial_send_byte() just calls usart_putchar() from the ASF. that function is blocking (waits until the USART is ready, basically). so calling serial_send_long() calls the byte-update function 4 times, and each of those calls blocks for, what 8/baudrate? 64µs or something? pretty long time.

(print_dbg does the same, which is why events get dropped and things generally choke with lots of activity in debug builds. maybe it's time to go through and strip out a lot of debug prints.)

if we want to start making heavy realtime use of the UART, then maybe we should investigate asynchronous updates and an output FIFO... oy

[catfact]

looking at datasheet, seems straightforward to set an interrupt for TXREADY on the USART...

so, it could go something like this:

// write function             
serial_write(char b) {
  // use the lowest-level ASF function to attempt a write
  // returns USART_TX_BUSY or USART_SUCCESS, based on tx ready bit
  if (usart_write_char(b) != USART_SUCCCESS) {
    // usart isn't tx-ready, add to output FIFO for later
    serial_tx_fifo_add(b);
  }
}

// TXEMPTY handler
uart_tx_irq() {
  // if there's anything in the fifo, send it now 
  if(!serial_tx_fifo_empty()) {
    usart_write_char(serial_tx_fifo_next());
  }
}

struct { 
  int idx_write;
  int idx_read;
  int count;
  char data[SIZE];
} serial_tx_fifo;

void serial_tx_fifo_add(char b) {
// DISABLE TXREADY INTERRUPT
   // add data, update idx/count...
// ENABLE TXREADY INTERRUPT
}

char serial_tx_fifo_next() { 
   // update idx/count, return oldest data...
}

bool serial_tx_fifo_empty { 
   return (serial_tx_fifo.count == 0);
}

[catfact]

Set up a debug variable to print number of serial events since last complete message, and the chars in that message were going through the event loop one at a time (though reading serial.c properly this doesn't seem to be the intention). <

well, it looks like the intention is indeed to raise an event on each incoming byte:
https://github.com/tehn/aleph/blob/dev/avr32_lib/src/interrupts.c#L246
each byte is stuffed in a ringbuffer. the event handler can check the ringbuffer to see what bytes and how many have been received since it lasted handled the serial input event.

i agree that this doesn't seem maximally efficient... . i guess at the time it seemed like the best way to leave it up to the application to implement whatever serial protocol it wanted. and it doesn't assume that each event will necessarily be serviced before the next interrupt. (which is good...)

is this what you mean, or something else?

[ghost]

Apologies was sure I'd pushed that code last night (pushed now). This is the function that sends all serial bytes (copied from op_serial.c)
https://github.com/rick-monster/aleph/blob/dev/apps/bees/src/ser.c#L23

i agree that this doesn't seem maximally efficient.....
is this what you mean, or something else?

yes that's what I meant - was blaming event buffer overflow on serial rx generating too many events, also thinking the machinery of event loop itself could be burning up precious cycles.

Interesting that 50ms is the magic number for updating dirty regions, whereas serial comms blow up instantly as soon as I reduce the sleep between messages below 50ms!

Could be a total coincidence, but is it possible that setting dirty flag twice without redraw triggers a crashing bug? Will try to find where that 50ms is defined, try tweaking it, see if it affects things.

The tests results I posted were a release build (make clean; make -k R=1). Release-build/debug-build didn't seem to make much difference how hard I could push the USART. Should repeat that experiment taking careful notes to make sure I didn't screw up somewhere...

EDIT (wrong times/rates again & corrected - too much/not enough coffee/sleep probably)

[catfact]

Interesting that 50ms is the magic number for updating dirty regions, whereas serial comms blow up instantly as soon as I reduce the sleep between messages below 50ms!

that is suggestive...

Could be a total coincidence, but is it possible that setting dirty flag twice without redraw triggers a crashing bug? Will try to find where that 50ms is defined, try tweaking it, see if it affects things.

.. but i don't think it's that, exactly; we do many, many drawing calls between refreshes already. that's kinda the point. (for example, each separate string that we draw sets a dirty flag, and we have lots of strings per menu page.)

here's where the screen timer is set, if you wanna try tweaking the period or disabling it entirely:
https://github.com/tehn/aleph/blob/dev/apps/bees/src/app_timers.c#L149

[catfact]

(moved some additional comments about screen rendering to new issue #252)

[ghost]

Another empirical observation made whilst hacking around last night. adding 100us sleep in this space:
https://github.com/rick-monster/aleph/blob/dev/avr32_lib/src/serial.c#L23
results in a catastrophic event-queue-overload fail even when sending a single 7-byte-long serial command.

Is that to be expected? Was kind of surprised adding this delay should set off a chain reaction totally overloading the system...

[catfact]

serial_store() is called from the UASRT rx interrupt, so yeah, its execution time is pretty critical.

[ghost]

If rx interrupt execution time is super, super critical another quick test to try this evening will be to shift everything including the calls to usart_read_char into the event handler in apps/bees/ser.c . So kEventSerial becomes a simple flag "hey main loop, there is new serial data, go check it".

Got that experiment to try, plus the test hitting usart hard in play mode without displaying that param/bees-in. Hopefully all this random thrashing will eventually result in enough understanding to get things moving forward again...

[catfact]

hm, i don't quite follow...

kEventSerial becomes a simple flag "hey main loop, there is new serial data, go check it

... that's exactly what it is (or should be.) the rx interrupt handler should do nothing but:

• stuff a byte in the serial RX ringbuffer.
• raise the event flag.

this is irq_usart(), which calls serial_store()
https://github.com/rick-monster/aleph/blob/dev/avr32_lib/src/interrupts.c#L245
(and no, i'm not sure why these are two functions...)
this behavior is defined in avr32_lib, it's not app-specific

the event handler, handle_Serial(), which calls serial_process(), is defined by the app, and it isn't called from an interrupt, it's called from the main loop when an appropriate event comes up:
https://github.com/rick-monster/aleph/blob/dev/avr32_lib/src/main.c#L338

so, spending too much time in an event handler is not great, because more events will come in and eventually fill up the queue. and if an event handler uses a critical section (the somewhat-misnamed app_pause() and app_resume()) then it will also preempt interrupts during that section, so those blocks should only be used to (quickly) access data structures that are modified by interrupts (like the ringbuffers or event queue) or to do stuff with peripherals that is timing-critical (like blackfin or filesystem interactions.)

an interrupt handler effectively constitutes a critical section automatically.

(sorry if all this is overly obvious or pedantic, just trying to be clear.)

do you mean my proposed TXEMPTY interrupt handler above? but that's a different beast: usart_write_char() rather than usart_put_char(). the first one doesn't block, it just returns success or failure. the second one calls the first one, blocking and calling repeatedly until it succeeds

[ghost]

nope haven't got my head round your tx proposal yet so can't comment on it, though I can remove all tx from this stress test. The change I'm proposing is move this stage:

stuff a byte in the serial RX ringbuffer

Into the main loop when the event gets handled and if there are more bytes available, keep stuffing (like what's currently in serial.c). Also instead of stuffing them into serial RX ringbuffer, call my function recv_char directly:
https://github.com/tehn/aleph/blob/dev/apps/bees/src/ser.c#L384

So the interrupt code becomes:

void serial_store() { e.type = kEventSerial; event_post(&e); }

Can't explain why I have a weird hunch this might help - I'm probably totally wrong. Quick enough to try and see.

[catfact]

ok, i see, sorry for the rant... let me think for a moment...

[catfact]

... oh, of course. because we're not actually getting the data in the interrupt directly.

yes that makes sense. we don't need the extra FIFO because the usart driver already has one. i forgot that we can keep reading bytes from the usart as long as they exist.

in this regard its different from our USB drivers, upon which the serial driver was apparently patterned. in those, we have to be careful to store every byte we receive and process them in the correct order.

[ghost]

OK so thanks very much for taking the time to explain all the issues surrounding interrupts.

So the results of the first set of experiments (nothing drawing to the screen, no net running -just sitting in play mode) are:

• with the hack to serial.c (no interrupt until message end char) can blast serial messages absolutely flat-out from lisp at the device without failure (1000 messages in 0.5s) . Also tx 17 chars of debug for each rx message during this test.
• Tried reverting the hack to serial.c so that one event triggered every received char. In this case the maximum speed before bees explodes is 1 message per 800us. Same debug tx.
• defering calls to usart_read_char to main loop, bypassing the extra ring buffer, blows up bees on first serial message. Definitely not the right direction!

Note: the serial message used for the test was a DSP param set command with random 16 bit data (7 bytes total)

[ghost]

And the experiments combining UI events with serial stress:

• each encoder hooked up to a dsp param in play mode
• all performed with the optimal configuration for serial rx as described in the first bullet point in previous post and with debug tx during the test.

Three tests:

• frantic encoder-twiddling with on-screen readout for encoder params - bees explodes at only 10 serial messages per second
• frantic encoder-twiddling without on-screen readout for encoder params - full stress test (10000 messages in 5 seconds) without blowing up.
• Navigating bees menus during serial stress (not play mode) - bees explodes at only 10 serial messages per second

[ghost]

experiment to test whether screen refresh rate set in init_app_timers correlates with maximum screen update rate we can trigger using UART.

Selecting play screen, using same test serial message from earlier experiments (DSP param update random 16 bit value) & displaying the param on screen. Gradually increase the rate of serial messages until bees explodes.

• with screenTimer set to 50ms the maximum permissible rate is one param update over serial every 56.5+/- 1.5 milliseconds
• with screenTimer set to 100ms the maximum permissible rate is one param update over serial every 110+/- 10 milliseconds.

Coincidence? I think not!

[catfact]

hm, well it's unclear what the exact crash mechanism is, but clear enough that when redrawing the screen is eating up lots of time on the main loop, Something Bad happens... maybe an RX overrun, because screen redraw takes place in an especially big critical section. (maybe it doesn't need to; that would be nice...)

defering calls to usart_read_char to main loop, bypassing the extra ring buffer, blows up bees on first serial message

huh! that's unexpected... but maybe shouldn't have been, because now that i am actually properly looking at the driver and datasheet, i see that the usart peripheral doesn't have its own buffer, and though it can be explicitly connected to a DMA channel (PDCA in atmel-speak), the ASF driver isn't doing this. it is just checking the data byte of the Receive Holding Register (RHR). this register is written when a char arrives, raising RXRDY at which point we get an interrupt, since we've set it that way: https://github.com/tehn/aleph/blob/dev/avr32_lib/src/init.c#L143

if RHR is written with RXRDY already set, it gets an Overrrun Error (setting OVRE bit.) not clear (to me, yet) what other consequences that has. we can enable an interrupt for it if we want. (add AVR32_USART_IDR_OVRE_MASK to the IER register in the line linked above.) then in the IRQ we would want to check for overrun, and i guess clear the error bit by setting RSTSTA in the control register.

hmm.. usart_read_char() already checks for the OVRE bit. returns USART_RX_ERROR instead of USART_SUCCESS but doesn't distinguish between overrun, parity, or framing.... anyways looks like DEV_USART->cr |= AVR32_USART_RSTSTA_MASK would clear the overrun.

given all this, i'm not sure why there is a while() loop in serial_store(). it doesn't seem like there could possibly be more than one character to read.

for TX, we should definitely be writing to a fifo in the main loop and dealing with it from the TXREADY interrupt (not TXEMPTY like i said above.)

[catfact]

hm, can't argue with that.. but for example, i can run a METRO at 10 ms triggering a render to the play page. so it's not simply a bug relating to multiple renders between screen refreshes.

[ghost]

just performed a sanity check taking serial out of the equation. connecting encoder to metro period, metro tick to DSP param, displaying DSP param on screen. Reducing metro period to 5 ms is not an issue with screenTimer set to 100ms.

So I believe it is the case that the crash is caused not by the draw code, not by the serial code, but by some conflict between the screen & serial code. Although I didn't yet fully absorb all the earlier information about interrupts, I noticed earlier that a fairly large chunk of the screen code here is wrapped in the app_pause() app_resume () - could that be relevant I wonder?

https://github.com/rick-monster/aleph/blob/dev/apps/bees/src/render.c#L145

[catfact]

at this point i'm going to put my money on some consequence of RX overruns in the usart. because while screen refresh is happening, usart interrupts can't be serviced.

gonna try taking the screen redraw out of the critical section, just for giggles... because i don't see anything particularly wrong with interrupting it.

[catfact]

another thing to try might be promoting the usart irq priority from UI_IRQ_PRIORITY to SYS_IRQ_PRIORITY, which would prevent it being suppressed by app_pause()/app_resume().
https://github.com/tehn/aleph/blob/dev/avr32_lib/src/interrupts.c#L304

[ghost]

randomly tried ripping out the interrupt protection from that bit of render.c . Bees can now easily take 200 messages per second (with the 100ms redraw rate still in there).

Totally stoked right now thanks for all the help & advice!!! just going to try the IRQ priority as another potential solution - maybe I did see a little screen glitch at one point though can't reproduce that now...

EDIT: changing the IRQ priority doesn't give a drastic improvement. Probably too tired though and should repeat all experiments at some point over the long weekend to be safe....

[catfact]

i don't know if it seems like a good idea for usart interrupt to be permanently un-preemtable, just thought it was something to test. at this point i'm most curious about what actually happens on an RX overrun. if it's really bad, then yeah we should make sure those interrupts always make it through.

but i, too, went ahead and removed the critical section from the screen update calls in bees render.c. seems to work fine and i can't see it being a problem. worst thing, maybe 'tearing' or other redraw glitches if rendering calls come in and change the buffer in mid-refresh. but anything that does that should be in the main loop... and this seems like a small price to pay for better responsiveness to realtime input...?

[ghost]

Agreed then - remove redraw the critical section from screen update and ignore odd redraw glitch for now. I have no idea how to find out more about what's happening on RX overrun, though I did see a case when I was Denial-of-Service attacking bees with serial bursts and saw it mis-interpret a couple of the messages - some bytes must be getting dropped somewhere.

Attempt to repeat this experiment did result in bringing down bees but that's running ~ 1k messages per second flat out - I'm satisfied serial is now robust enough. Aspects that certainly still need attention on this issue:

• somehow clean up that dumb hack in avr32_lib serial.c where I throttle messages based on knowledge of bees serial protocol (how?)
• plumb in the serial commands for repatching bees network (straightforward I think)

[catfact]

hm, yeah that's a quandary. most of the time, delimiting by CR makes sense, but not all the time... even within the same app. for example, if you want to use the serial port to load blackfin executables in bees...

seems like we need to add a mode switch to turn off delimiting. and maybe make the delimiter value a variable, defaulting to CR.

we should also keep in mind the limited size of the ringbuffer... it's 64 characters right now. could need to be bigger if there were something like a forth-interpreter app (which i definitely want! some day)

considering rx overruns, i'm not seeing any reason they would cause a crash, should just produce dropped characters... will keep looking and try to isolate the crash condition.

[ghost]

ok so revising the points to close out this feature:

• add switchable delimiting on a serial delimiter global var to avr32_lib serial rx (COMPLETE)
• trigger main loop event on either delimiter or buffer 1/2 full. (COMPLETE)
• increase that ringbuffer to 256 chars (COMPLETE)
• plumb in & test bees patching commands (COMPLETE)

[ghost]

W.r.t the crash I have one question: Does bees recover gracefully from event buffer overflow once the stimulus which triggered overflow is removed?

I am going to try commenting this line:
https://github.com/rick-monster/aleph/blob/dev/avr32_lib/src/events.c#L90
Hoping the effect of this will to effectively flush the event queue on overflow, rather than attempting to hold on to the 1023 events in the ringbuffer.

EDIT: kind of an interesting experiment but inconclusive and doesn't seem to solve the problem. I don't want to duplicate labour if you're already looking for the crash condition so moving back to those other points...

[ghost]

other points are done now - patching over serial works!!!! - remaining niggles with serial comms:

• tx should really have a FIFO
• why does bees totally crash under really brutal combined serial & UI stress test?

[ghost]

Just increased some buffer sizes, did some more experiments. Gory details make for pretty tedious reading material but the upshot is I'm now sure event queue overflow is merely one of the symptoms of the crash.

Also made a breakthrough in demonstrating that backed-up Tx is also not the problem. Totally disabling serial Tx makes absolutely no difference to the crash. The problem seems totally unique to flat-out serial param or input triggering, when those params/inputs are displayed in play mode.

Since I cannot easily figure out how to hook up the callback on Tx-ready and there's some evidence a Tx buffer will not prevent crash on serial overload I think time for a pull-request. Quick dash to put together a cmd-line utility for hot-swapping modules over serial then I'll request the merge...

[ghost]

Calling this 'done'. Might re-open to add a 'special' serial op for my host software...