search

keyboardio / Kaleidoscope-Bundle-Keyboardio

A Kaleidoscope distribution for the Keyboardio Model 01 and other keyboards.
Other
17 stars 24 forks source link

model01: Use a modified CDC.cpp #7

Closed algernon closed 5 years ago

algernon commented 5 years ago

At least for OSX, we need to slow down Serial writes. The easiest way to do that is to add a delay after each write, and to accomplish that, the easiest is to modify CDC.cpp. Without this, on OSX, we end up writing too fast (and changing the baud rate does not help, no matter in which direction I change), and end up losing data.

This is not a great solution. It's not even a solution, but a workaround. But it works for now.

obra commented 5 years ago

Can you recast this commit as checking in the original file and then checking in your change as a separate commit?

algernon commented 5 years ago

Sure, will do that in a bit.

algernon commented 5 years ago

Updated

algernon commented 5 years ago

Having made all communication go through Focus (keyboardio/Kaleidoscope#477), I made an attempt at adding the delay to Focus.send(). That... doesn't scale well, because .send() is inlined. Thus we end up with not calls to Focus.send(), but calls to Serial.print and delay. With my own sketch, this is over 200 bytes extra, and each call to Focus.send() will adds yet another delay call.

I could redo keyboardio/Kaleidoscope#477 in a way that .send does not get inlined, but that will also add significant PROGMEM costs (though less than 200 bytes, but still too much), would use more memory, and would be a little bit slower too.

I'm afraid that unless we figure out the root cause, and can fix that, this delay in CDC is our most efficient bet. :(

obra commented 5 years ago

what about forcing noinline?

On Nov 23, 2018, at 3:15 AM, Gergely Nagy notifications@github.com wrote:

Having made all communication go through Focus (keyboardio/Kaleidoscope#477), I made an attempt at adding the delay to Focus.send(). That... doesn't scale well, because .send() is inlined. Thus we end up with not calls to Focus.send(), but calls to Serial.print and delay. With my own sketch, this is over 200 bytes extra, and each call to Focus.send() will adds yet another delay call.

I could redo keyboardio/Kaleidoscope#477 in a way that .send does not get inlined, but that will also add significant PROGMEM costs (though less than 200 bytes, but still too much), would use more memory, and would be a little bit slower too.

I'm afraid that unless we figure out the root cause, and can fix that, this delay in CDC is our most efficient bet. :(

— You are receiving this because your review was requested. Reply to this email directly, view it on GitHub, or mute the thread.

algernon commented 5 years ago

Didn't try yet, will do so later tonight.

algernon commented 5 years ago

Forced noinline for all send methods: +392 PROGMEM with my Atreus sketch, +684 with my Model01. Forcing no-inline only on the non-templated ones results in +30 PROGMEM only, which would be manageable. Adding delay(5) to the mix results in +254 PROGMEM.

This PR is a constant +28 bytes. Seems like we exhausted most other options... :/

obra commented 5 years ago

This makes me really unhappy, especially since it only works for the Model 01 and isn't a general solution. But I'm ok with it being merged for now.

I still don't like it.

algernon commented 5 years ago

We'll figure out something better before 2.0. I... have a couple of ideas, but it will take a while to explore them, and I'd rather not hold up Chrysalis usability on OSX until then.