Allow all TXi knob and inputs to be queried via i2c

[ngwese]

The current ii.txi.* functions are a proof of concept and only allow one to query the first parameter knob on txi. At a minimum it is desirable to be able to query the raw txi 'param' (knobs) and input values for all four params and inputs.

The TXi supports also supports scaling/quantizing of values - something which is easily handled directly on crow thus exposing that feature set is likely a secondary concern.

[trentgill]

would love for someone to look further into this. from memory, the reason why it's difficult is the txi packs multiple params into a single byte, so crow needs to understand how to unpack those bytes (and hopefully roll them into the descriptor lang)

[simonvanderveldt]

@trentgill Do you know if something was added to Teletype to support those messages? Or was it already a standard/convention that was being used there?

[trentgill]

@simonvanderveldt On teletype, all the devices have a file that manually handles this. As opposed to crow which tries to use a unified method.

The telex helpers that do the shifting etc starts here https://github.com/monome/teletype/blob/22d6ce897e32d98d327144c8550e21b97dde257f/src/ops/telex.c#L356

[ngwese]

Okay after a bunch of experimentation and reading through code I'm not seeing a path to supporting this that doesn't require some refactoring of how message signatures (ii_Cmd_t) are handled throughout the code.

The central challenge is that the telex module i2c protocol breaks two expectations that the existing generic code has. The first crow expectation is that the i2c address of the device on the wire is the address contained in the lua ii descriptor code. The TXi and TXo modules (on teletype) expose parameters/ports to the user such that the asking for an input or output number is transparently mapped to one of potentially many modules. On TXi the first module has inputs 1-4 while a second TXi would have inputs 5-8. The way this is accomplished is mapping inputs/outputs across a range of i2c device addresses and computing the address based on the channel number. For crow with a single device attached (at the default address) this isn't an issue - if multiple devices are on the bus then the address for the i2c call would need to be different than what is in the lua ii descriptor. More of why this is a challenge in a bit.

The second expectation in the generic crow i2c logic is that the packets are of the form:

• byte[0] = i2c address
• byte[1] = command
• byte[N..] = additional command specific args (such as which channel, input, voice to operate on)

In the TXi case the get command along with what to get ("param" vs "in"), which channel, and whether it should be raw, quantized, or scaled value is all packed into a single byte placed in the command position (byte[1]). The lua ii descriptor logic expects the command value for the parameter to be constant much like the address. To communicate just support get requests from TXi the command byte must be computed.

The first obstacle to supporting the TXi based on my investigation is - where should the logic to do address and command byte computation live and how is that connected to the ii/txi.lua descriptor file? After searching for alternatives the most straightforward path was to allow the lua descriptor files to optionally specify a get_impl and set_impl property which caused the ii code generator to use an alternate lua function other than the generic ii.get(...) and ii.set(...) functions used. These alternate get/set functions could then be implemented directly in lua or C (I chose to do it directly in C). This approach preserves the existing generated helper logic while allowing a custom _txi_get function to to the address and command byte computation. So far so good...

...until one goes deep into the i2c stack. The ii descriptor code does a lot of nice work to generate static data (at the C level) which describes the type signature for outgoing i2c packets and related return values. The static encoded type signature data (described in via the ii_Cmd_t generated code) is looked up in the lower level functions to send/receive data on the i2c bus. The Cmd/type signature is unfortunately looked up via ii_find_command(address, command) where the address and command arguments are the literal values appearing in the lua ii descriptor file. Since the TXi protocol encourages address computation and requires packing the command byte with additional information the type signature cannot be looked up by the lower level code.

My first ugly attempt added "effective_address" and "effective_cmd" parameters along side "address" and "cmd" to the mid level functions such as ii_query. The ii_query goes on to call the low level I2C_LeadRx function which stashes away the address and cmd into an internal state global which the I2C_Lead_RxCallback function then uses to lookup the ii_Cmd_t type signature again. These lower level I2C_* functions ultimately to the packing and unpacking to the too have to keep track of the two different addr/cmd pairs that the high level code is maintaining - one pair representing what has to be put on the wire and the second pair to lookup the type signature for packing and unpacking of the i2c payload.

I'm at a crossroads - I could continue to weave the multiple addr/cmd pairs through the code (which seems rather ugly) or pick a different route. What I've started to explore is a possible refactor where:

• the ii_Cmd_t structure becomes more public and is accessible to the mid level ii.c code (not just in the low level i2c.c code)
• add derivatives of ii_query and ii_broadcast which have where the type signature (ii_Cmd_t*) is explicitly passed in; this would allow the custom get/set handlers to manage the type signature lookup themselves
• have the low level I2C_* functions which do packing and unpacking take the ii_Cmd_t signature as an argument instead of using ii_find_command(...) directly
• store a pointer to the ii_Cmd_t structure for the currently outstanding request in the i2c global state so that the callbacks don't have to lookup the type signature again in order to unpack the return value(s)

I'm going to continue down the refactor road as it seems the most clean even if it ends up being a bigger change. @trentgill - I'm curious as to your thoughts.

Trying to make thing more decoupled from assumptions about i2c device address seems like a good thing if the long term goal is to support multiples of a module (like teletype supporting multiple ansible modules as output expanders)

[simonvanderveldt]

Wouldn't it be better to update TXi/TXo to support the standardized message structure?

I don't really understand why the I²C address needs to be computed, couldn't that only happen on teletype and we'd address it directly?

[ngwese]

Changing the TXi/TXo firmware is certainly an option - it initially seemed like a steeper hill to climb given the larger installed base of TX* and teletypes.

The i2c address computation is not really that unreasonable and is something which I'd imagine will come up in one form or another for any module where there is the goal of supporting multiple on the same bus. Even if the device number became an argument like ii.txi.get('param', <device>, <channel>) or ii.txi[<device>]('param', <channel>) that still doesn't eliminate the need to either:

• burn space with redundant descriptors (one for each device)
• decouple the requirement that the i2c address on the wire is part of the key into a switch statement used to look up the type signature for the call (ii_Cmd_t)

The packing of multiple values in the command byte is the more debatable choice - if it were me I probably would have done the TXi protocol a bit differently (simplicity of multiple bytes vs compactness/speed of the packed form). At the end of the day it is from my perspective a design question which involves the tradeoffs between:

• forcing all devices to conform to what can be expressed in the ii descriptor DSL today
• expanding the DSL to support enough syntax to express the shifting/math/logical operations needed to implement the cmd byte packing
• expanding the DSL/generator logic to allow the inclusion of arbitrary lua code to do the packing/unpacking (this is possibly a rewrite of the generator logic to work off AST representation of lua code instead consuming the descriptor as lua data and generating code via string concatenation)
• some hybrid where as much of the existing structure is maintained as much as possible in addition to specific logic in the firmware for more "exotic" cases like this protocol (my head is here at the moment)

[simonvanderveldt]

Regarding the addressing, I'd expect ii.txi[<device>]('param', <channel>) to work since this is in line with how you handle the multiple inputs and outputs as well.
But that would obviously need some way to map the <device> index to the correct i2c address.

@bpcmusic Do you have any suggestions/input on this? I don't know if there was a specific reason for packing multiple values into the "command" byte?

@tehn What do you think?

[trentgill]

My inclination is the same as greg’s. Keep what’s there as much as possible because it works for a lot of modules & encourages uniformity for future products. Then extend the DSL to allow custom code to be shimmed in at the lowest level possible. Personally I hope we can get away without changing ii.lua at all, instead having the changes exclusively in C (for both speed & memory utilization reasons). I’d also like to avoid using a real AST, instead prefer staying with the current lua-as-data approach. It makes debugging & file validation far easier / less error prone (i should make a script that validates the files & prints the generated code). I think we should support a single module first before dealing with the question of how to represent multiple devices. That’s a bigger question and can be broken off as it’s own topic (affects other modules: er301, w/ etc). After we solve that generally it makes sense to look back at tx* modules and determine a technical solution to the computed address. I’m really happy to help on this front! Let me know if there’s somewhere i can help best. The code generating scripts are really sloppy and i’ll prioritize refactoring them if that would be a help!

…

On 8 Oct 2019, at 04:13, Simon van der Veldt ***@***.***> wrote: Regarding the addressing, I'd expect ii.txi[<device>]('param', <channel>) to work. But that would obviously need some way to map the <device> index to the correct i2c address. @bpcmusic Do you have any suggestions/input on this? I don't know if there was a specific reason for packing multiple values into the "command" byte? @tehn What do you think? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.

[trentgill]

Before going full tilt on tx* support, should we do a quick roundup of other i2c devices and see if this is an isolated situation, or if there's other code-hacks we're going to have to add?

A couple things we've seen so far:

• Ansible indexes seem off by 'some number'
• ER301 PR isn't working. i know brian from OD referenced the tx* modules when implementing it, and also noticed an off-by-one error being accounted for on teletype (so we'll probably have to hack that into crow as well :( )

Otherwise there's 16n which i don't have, but what other devices?

[ngwese]

@trentgill - Regarding the "Ansible indexes seem off by some number" comment. Are you referring to the addressing of individual CV and TR outputs when ansible is in teletype/expander mode?

If so then this looks like possibly just a "C" 0 based vs "Lua" 1 based thing. The ansible firmware is using the second byte in the i2c packet as the channel (for ops which have a channel) and it appears that is assumed to be 0 for the first CV/TR. Is this just a point of confusion relative to JF? My vague recollection was that JF i2c commands used channel/voice numbers of 0 to address all voices/channels so in practice interacting with JF voices would feel 1 based.

[ngwese]

Confirmed ansible in teletype/extender mode is "off by one" for channel numbers as compared to teletype.