#85

this is wonderful progress, and despite being not fully to your goal, i think a merge would be good-- i’m going to start back in on TT code in earnest very soon and if i understand the structure you’re going for with abstraction, i can likely help finish up the task as well!

5 Likes
#86

Cool, I’ll try to get a PR in by Thursday or Friday. I’d like to have a go at finishing the code that’s uncommitted on my computer… if don’t get time I’ll put the PR in anyway.

3 Likes
#87

was hoping to get some opinions/thoughts on further enabling variable length i2c messages in the teletype codebase. (these comments refer to the refactored ops branch that @sam has been working on.)

while it is true that @tehn added support for a variable-length package to be sent over i2c, the eventing framework that these messages are passed through in the teletype is done using the event_t struct which limits the data size to s32.

for the output expander’s purposes, we could get away with adding just a wee bit more data room for the expanders as they exist today - that is, the messages for the output expander will most likely look like:

[ u8 - expander address ] [ u8 - output # ] [ u8 - command ] [ s16 - (optional) data payload ]

but, one could imagine that in the future wanting to send more complex information. (for example, i believe someone mentioned sending a block of pattern data at one point.) on the other hand, most of the handlers don’t even use the data parameter - so overhead here would seem like overkill.

there are a number of ways to do this, but no individual one is standing out as ideal. any opinions?

1 Like
#88

there are certainly a lot of ways to do this. @zebra and i worked on a working specification for a broader i2c communication scheme that i’d like to implement into the TT ecosystem, and i’ll be working on this in the coming weeks.

we’ll start publishing information about it once we do some preliminary testing.

at the moment i’d suggest simply making your hardware and basic code work. the “old style” i2c method of static addressing will work fine as a test platform.

1 Like
#89

sure. output expander is easy to test this way (i’d already hacked it in).

eager to see the specification and explore a custom operator to speak with multiple expanders. also, i’m very curious about your preferred implementation for pulling data in so I can build out the input expander functionality.

thanks!

b

#90

Here we go then… let me try to summarises what changes I’ve made, and what’s left to do.

  • The ‘pure’ algorithm code now resides in the src directory, while all code related to the module is in module. Hopefully this will allow main.c to be broken up into smaller files and for it to be obvious what those files concern. There are definitely things in main.c which really belong in src, things like inserting and deleting lines from a script, which currently make quite intrusive modifications of the arrays, rather than calling a higher level function like e.g. tele_script_insert.

  • The algorithm state is now represented by 3 structs, scene_state_t, exec_state_t and command_state_t, see state.h for details. The only static variable left in the algorithm code is a static scene_state in teletype.c, at some point soon that will be removed and all the functions will expect a scene_state_t as the first argument (the ops already do).

  • Ops have now been broken out into separate topical files. This is to keep like minded code together and make it easier to see ways to reduce code duplication. A good future example of this will be when I (or someone else) creates PN.* versions of all the P.* ops.

  • Scripts are now executed using run_script and run_command which take care of creating a valid exec_state_t (process is still there doing the actual business). This means that the behaviour of IF, ELSE, and ELIF have changed subtly. But on the plus side it should be trivial to add a STOP op to halt the current script.

The big things left to do are…

  1. Get a grip on main.c
  2. Get rid of the static scene_state
  3. Rewriting the parser to not use strcmp (probably via ragel)
  4. Build a better user documentation system

To the best of my knowledge everything still works properly, but I haven’t been able to test it a lot. Given how much has changed I find it hard to imagine that some bugs haven’t slipped in.

The code itself is still a WIP, but I’m probably not going to have much time to work on it for the next month. So it seems like a good idea to get it merged in now. Any questions please ask either here or on the issue.

Just a quick thought on the i2c stuff, I’ve never really used it before and I know very little about embedded programming outside the teletype codebase, but…

I’d say it’s important to over engineer the protocol, given how many different modules (and alt firmwares) will be using it, it’s going to be very difficult if we need to continually update them every time it changes. Even more important, if we do need to change in a breaking way, there needs to be a way for those messages to be rejected by devices running older codebases. It would be terrible if someone was to buy a new meadowphysics, connect the II bus and then have their teletype constantly crash due to protocol incompatibilities.

3 Likes
#91

doing a big review today of this! looks amazing. thank you so much. it’ll take me a bit to fully wrap my brain around what you’ve done here.

re: i2c, @zebra and i are definitely accounting for very broad use cases. we’ll deal with firmware updates to keep other things up to date and deal with message breakage.

1 Like
#92

thank you for this huge effort! will be reviewing the PR / commenting sporadically this week.

re: i2c - agreed on over engineering it and accounting for potential future use cases. would be great to have the ability to pass different number of parameters for different commands, and to be able to pass an array of values as well [such as pattern data].

i think back compatibility shouldn’t be a huge issue? since all the existing firmwares could be updated prior to that, and then when folks update their teletype it’s easy to update all the modules at the same time? but perhaps the updated protocol could start with a byte that is not currently reserved for anything so any modules running the old firmware could just ignore the new II commands [assuming they can deal with more data being sent].

1 Like
#93

I’m about for the next fews days if anyone does have any Qs. After that response times might be a bit slow (till late June).

The sketchiest bit of the code is the most recent, the script array getters and setters, basically it’s unfinished, eventually it will be replaced with more semantic functions (e.g. tele_script_insert), but replacing the direct array access with functions seemed like a useful stepping stone when I was refactoring.

On the i2c front, I’m guessing that @zebra has had some experience with this via the e bit of the Buchla 200e system? That’s i2c based too isn’t it? Practical experience trumps theoretical knowledge…

#94

the 200e has a lot of i2c. we’ve been discussing it.

i’m wading through the PR and haven’t come up with questions yet, though i have much more to cover.

1 Like
#95

started looking into https://github.com/monome/teletype/issues/16 [key behavior: repeat on hold] somewhat selfishly wanting to specifically add support for holding backspace while editing, and right now it’s implemented separately for each supported key in handler_KeyTimer.

it’d be trivial to add it for backspace only, but thinking it might be good to refactor it to support any key hold by extracting key processing into a separate function and then calling it from both handler_KeyTimer and handler_HidTimer, so in essence the latter will imitate a key press when appropriate. any concerns / objections for such change?

#96

made the change as described: https://github.com/monome/teletype/compare/master...scanner-darkly:dev
did a quick test and everything seems to be working okay. will do a pull request after i test it a bit more.

3 Likes
#97

thank you for breaking out process_keypress as i was going to do that myself!

#98

going to refactor it a bit, remove global variables for the modification keys and add a parameter for specifying when it’s a held key as in some cases repeated key should be ignored (like saving a scene, for instance).

realized makefile for clang-format is Mac specific, is there a way to write in a way that would autodetect the OS and run a different command on Windows?

#99

https://github.com/monome/teletype/pull/44

finished key repeat implementation. reformatted earlier code a bit (got rid of global key variables in process_keypress) and also key repeat will now be ignored for writing and reading from flash. key repeat will still work for triggering scripts - thought it might be useful to have it work that way…

also found a couple of minor bugs from @sam’s refactoring, pattern length wasn’t getting properly updated in some cases, and [ and ] didn’t update values when in TRACKER mode, that’s fixed now. also made a change to ENTER behavior when in TRACKER - it extends pattern length when pressed on the last step as before, but now it will also go to the next step regardless of where it’s pressed, not just when on the last step - this seems like a good thing to have.

and discovered a few key combinations i didn’t know about! CTRL and CTRL+SHIFT in TRACKER mode specifically is rad, very handy to be able to set pattern values with the PARAM knob…

5 Likes
#100

thank you for this! and yes good suggestions with the key behavior.

1 Like
#101

Was just working my way through the implementation to support negative voltages for my output expander and found a little inconsistency in the way voltages are handled in the Teletype code. From the web page:

These are typically used with CV output, since CV has a fine granularity (14 bit), full range which is 0-16383.

So V 10 (ten volts) is 16383. V 1 is 1638. N 1 is 137, for a semitone. This way we can use readable (small) numbers and then translate them to CV easily.

But - when I look at “V 10” I’m seeing 16384. A quick peek at the table backing the conversion and you can see the lookup for V 10:

const int table_v[11] = { 0, 1638, 3277, 4915, 6554, 8192, 9830, 11469, 13107, 14746, 16384 };

I bounded it for the operator for the output module, but we might want to take a quick peek at the tables to ensure that the values are proper. Rounded, 16383 spread across the (11) values would be:

const int table_v[11] = { 0, 1638, 3277, 4915, 6553, 8192, 9830, 11468, 13106, 14745, 16383 };

This change also corrects the range for the VV operator as well (as it is based off the V table).

b

1 Like
#102

@scanner_darkly thank you so much for doing this!!
was just going to report a “bug” when pressing ctrl :smile:
forgot about that too!

hex file seems to be half the size of before? can this be true? is that @sam’s magic or will I be hitting a wall because of wrong compilation… so far everything seems to be working.

#103

not sure about the size - looking at the official releases v1.1 is ~597k, v1.2 is ~227k and the latest code built is ~242k, but both 1.1 and 1.2 point to the same checkin in git, so hard to say what was changed. i do seem to remember one of the releases included the default scenes with it, maybe that’s why?

#104

ah yes was comparing with 1.1, maybe the asf diet? or the scenes…

(nevertheless, big cheers to @sam too for helping out so much here and laying the foundation for future developments!)

1 Like