Encoders connected to multiplexer inputs

[GioCC]

Since questions about this possible improvement pop up from time to time, showing that there is interest (often from SimVim users), and since I also believe it makes sense, I'd like to start with a few thoughts about it.

Firmware-wise, "feeding" an encoder with an input from a multiplexer would be easily feasible with very little penalty:
multiplexer inputs can be directly addressed rather quickly (just 4 DigitalOut() instructions per pin to set the address) and independently (i.e. you can change the multiplexer address in any moment).
Their impact on the speed of the polling cycle should be therefore very limited, regardless on where, on which pins of which multiplexer, they are connected.
The situation would obviously be very different for input shifters, where a complete and slow scan cycle (and intermediate storage of the results!) would be required.

A slightly more demanding modification is required on the Connector side.

Currently, on the Connector, the encoder inputs are simply defined by the Arduino pin number; with multiplexer inputs, in order to fill the selection list, a reference must be made to other devices (the multiplexers) which may or may not be present.
The program should first scan for the existence of input multiplexer devicess (and read on which pin they are connected), then insert their inputs as list items, assigning a special name.
For example, for all 16 inputs of the multiplexer connected to pin 23 this name could be something like e.g. "23/0", "23/1" ... "23/15".

In the encoder configuration message, for each encoder pin, the multiplexer pin (or "channel") number - the second figure above - would need to be added to the current message payload, to be used by the firmware as the multiplexer selector/address value.
The pin number already present would be set to the pin number associated to the multiplexer.

There might be side effects to consider, e.g. a situation where an encoder uses inputs from a multiplexer device which is subsequently removed, leaving the references invalid and possibly undetected.

[GioCC]

I quote a reply from @DocMoebiuz on the discord channel, adding some considerations:

the firmware tells mobiflight what devices are connected to it
but the available pins not yet... this is in the boards json

An encoder using multiplexed pins need not know anything different from the board json.
The presence of (at least) one configured digIn multiplexer is required to enable multiplexed pins on a std encoder, and that is already configurable; the encoder just takes the multiplexer input pin as its own.

and the firmware also would have to understand that there are additional pins

The firmware only needs to receive two more values in the configuration, ie the mux selector values for its two pins). If these values are e.g. 0xFF, the encoder simply skips the selector setting when reading pins, everything else is unchanged.

well then we could just add a multiplexer... it creates additional I/O pins and we could use those for any kind of devices

This is an (obvious) intrinsic precondition to use MuxEncoders. No need to automate the creation: if no digIn multiplexer is previously configured by the user, obviously an encoder can't use multiplexed pins.

we would only have to have a way to address the pins correctly in the firmware for all devices
because an encoder would have to know how to deal with a built in pin and with a virtual pin from a multiplexer

This is almost trivial, and inexpensive (at least for the case at hand), see above.

if the data pin is an analog pin we could also support additional analog inputs

Yes, a device for multiplexed analog inputs could be created just as done for digital inputs (clearly with its peculiarities).

and we could drive devices like a max7219 chip or other SPI stuff (i guess)
adding support for virtual pins might not require too many difficult changes on the Connector side.
and we should be able to use the same mechanism for other i/o expansion chips too

Indeed, but this is entirely another topic, which is already under discussion here; the first step is "shared driver pins for devices", then "SPI devices behind multiplexers".

[GioCC]

I realized I forgot to tag anybody, so this discussion might remain unnoticed... shoutout to @neilenns @elral @tigert @Pitj3 and anybody else interested

[GioCC]

It's probably useful to add this summary of the details of my proposal; I'm not sure if I might have missed something, so please let me know your comments.
(There may also be further optimizations, but they're beyond the point of this explanation)

• Connector, configuration panel of encoder device:
  -> The panel aspect and UI operation remains the same as the current one.

  IN the underlying code, when filling the list of available pins:

  • detect if any digital input mux is configured, and get their pin numbers
    (ex.: "Mux1" on pin 21 and "Mux2" on pin 33)
  • add entries for their inputs to the selection list for the encoder inputs
    (ex.: "Mux1.1", "Mux1.2"..."Mux1.16", "Mux2.1"..."Mux2.16")

  If one of these inputs is selected:

  • use the corresponding mux input number as the encoder input and add the channel number to a new field of the encoder config message.
    (ex.: "Mux1.7" as Enc pin A: Pin#=21, Channel#=7)
    For "normal" pins, Channel# is a dummy value (ex. 0xFF)

  Drawback: in the list, mux inputs already assigned to other encoders would appear.
  Solution: see "input mask" below.

  To avoid clutter, instead of adding mux pins to the same list, an additional drop list could be made to appear to select which (if any) of the available multiplexers should be used; the pin list would then be limited to 1...15.
  But this is just a different GUI setup for ease of use, nothing changes in the internal handling.

• Connector, configuration panel of digital input multiplexer:
  -> No change.

• Connector, Input configuration panel, encoders:
  -> No change.
  The encoder events generated by the FW are exactly the same regardless of where their HW inputs come from (which the connector ignores at this point anyway).

• Connector, Input configuration panel, multiplexer inputs:
  -> No change.

  Drawback: The user could still assign a configuration to an encoder-assigned input, "doubling" it as a button; this, unless it's found to make sense, should not be a big deal anyway, because the user will sooner or later realize that he's trying to connect two wires from different devices to the same terminal.
  (Besides, if the "input mask" (see below) is implemented, the corresponding events would never be generated.)

• Firmware, configuration data:
  -> Two more bytes should be added to the configuration message: mux selector values (or dummy values otherwise) for encoder pins A and B

• Firmware, operation:
  During encoder scan:
  -> if the selector value for any pin is not 0xFF, the multiplexer selector pins are set to that value. Everything else unchanged; the pin values are read, and perfectly normal encoder events are generated.

  During Multiplexer Input scan:
  -> No change at all.

  Drawback: On encoder-assigned inputs, normal button events are uselessly generated.
  Solution: see "input mask" below.

And this is a summary of the total impact:

• Total impact on user experience:
  Basically completely transparent, the only difference is that now multiplexer pins appear among the available pins for an encoder - as expected when this is an available feature.

• Total impact on performance:
  Almost negligible, FW only (a few bytes more in config data; a few instructions more for encoder polling)

• Total impact on code complication:
  Connector - a simple scan function added in the encoder configuration panel (possibly a few panel controls more for clearer presentation)
  FW - extremely limited.

End note - Input Mask improvement:

Ideally, the mux inputs assigned to an encoder should be registered in a mask (on the mux device) in order to:

• (Connector) tell the encoder config panel, and possibly the input config panel for multiplexers, that those input(s) are not available
• (FW) tell the FW that it's useless to also generate button events for that input(s)

[tigert]

The mask sounds like a good idea - especially if we want to add multiplexed analog inputs as well. As a user it works as expected if I assign a pin to a button or other deivce, that it is no longer available for use for other types of devices either.

[GioCC]

Exactly. An exclusion mechanism like this mask is practically mandatory, but since it is an implementation detail I described it as a footnote to avoid distraction from the main topics.

[elral]

First of all, sorry for my late answer, I was quite busy with my daily work. But it should be better now ;)

I thought long about the support of encoders via multiplexer, and I see still "only" one reason to go this way. As the main stream supported Mega has enough I/O's for encoders, buttons, ... the now supported Pico lacks on I/O's and beside the shifter registers support of encoders via multiplexer might be sensefull.
Sebastian mentioned for this case to introduce a new "device" which have it's own deviceID. It could be named something like "Encoders on Multiplexer", which could be extended on next steps to other input/output devices. I agree that going this way it makes it much easier/clear for the user to define what he wants to connect and define. It would be "only" required to define this new device and it would be clear which pins are available for encoders. On the same way extension of e.g. analog in could be done.
Doing it this way there would be no need to "rebuild" the list of available pins where an encoder could be connected.

On the firmware side this new device must be implemented and slight changes on the encoder routines are necessary to read in the pin status from the mux device. I assume that the changes are negligible.
However, we have to keep in mind that we are short on Flash and RAM for the "small" Arduinos. Addiitonally we have to consider that the encoders should be read in ever ~1ms (I will test in the next days to which time it can be increased). So even with this extension it might be required that the max. numbers of encoders (sum of directly connected to I/O's and connected to mux) must be limited.

/edit: I forgot to mention, that it would be benefitial first to implement sending the deviceID instead of the names to free up the RAM consumption. Also it could be benefitial to use the deviceID to access the defined multiplexer (but haven't thought in detail about this topic)

[neilenns]

Yes, the reason I want this is on Picos 😊

[GioCC]

Just out of curiosity... how much weight does the compatibility with SimVim/RSC have in your opinion, when considering whether to adopt a feature? Assuming of course features which could make sense for themselves anyway, not ones to be introduced just for the sake of SimVim compatibility at the cost of disrupting the operation of MF.

[elral]

Do you really think I want to "disruptive" the operation of MF?? And why does it make think so?

I wanted to express the complete opposite, keep it the same way for the user as up to now and as simple as possible. That's for me more important than SimVim compatibility.

[GioCC]

Sorry Ralf, but what ever made you think my question was related to your reply?
The meaning of my sentence was simply this: there are features that get in the way of the mode of operation of MF, there are others that don't.
Many features to introduce compatibility with a different system are more likely to be of the first type, and for these, I would expect that the answer to my question would trivially be "not very desired", so, useless to ask.
Therefore, I specified that my question was limited to the other kind of features, where the compatibilty would be an independent (and in a way, free or almost so) advantage.
The reason I asked was simply because, when discussing possible new features (when they are of the second type), I almost never see anyone mention compatibility with SimVim among the advantages, despite seeing lots of users constantly asking for that on discord; and my (I hope legitimate) curiosity is whether this is because this compatibility is of no interest, or simply because many of "core" Mobiflight users do not come from SimVim and therefore are not familiar with what could be useful to a SimVim user.
That's all there is and nothing more, I certainly didn't have your reply in mind when writing my question - and going back to it I don't see what, in your reply, could be considered any more "disruptive" than my own proposal (or any modification at all).

[elral]

Hmhm, because you wrote:

... have in your opinion ...

as direct answer to my thread. So sorry for this, I guess I have to take some more english classes ;)

[GioCC]

Well, it wasn't actually a direct answer, that's also why I didn't see the connection... but I see your point. Glad we cleared this. 😉👍
Also now maybe the purpose of my question is more clear.

[DocMoebiuz]

I think it's great to provide a migration path to users who are coming from SimVim. For me that's one of the main reasons we introduced multiplexers in the first place

We had already Output and Input Shifters, the 74hc4067 are just more widely used. At least that is my impression.

So it's a great alternative for other users too.

[tigert]

It's good if we can make life easier for simvim users while making useful things for ourselves.

What the pico lacks most is analog inputs (it has just 3 usable pins, fourth shows cpu temperature I think). Encoders also consume a lot of pins if you have a handful of them.

The special device type sounds like a good idea to keep the UI a bit more sane.

[elral]

What the pico lacks most is analog inputs (it has just 3 usable pins, fourth shows cpu temperature I think). Encoders also consume a lot of pins if you have a handful of them.

That's exactly I tried to explain, have different devices from the user perspective. One device for "encoders on multiplexer", one device for "analog on multiplexer" and so on as addition for the existing "buttons on multiplexer".

[DocMoebiuz]

I know I repeat myself when I say: ideally a I/O expander adds pins that are available then transparently for connecting any supported device.

We have already given thought to this and it seems like it's not trivial on the firmware side.

I think being able to add encoders to multiplexers would be a nice addition, but i am looking for a clear experience for the User during configuration and a clear handling by the MF connector.

At the moment I am not so happy with the way how the MultiplexerDriver is a very special device. It became part of the basedevice class and has to be treated specifically by the UI. That's why I would like to think of ways to enable the support for encoders without too much of additional extra treatment. Maybe just some refactoring would help to keep things less intertwined.

[GioCC]

I know I repeat myself when I say: ideally a I/O expander adds pins that are available then transparently for connecting any supported device.
We have already given thought to this and it seems like it's not trivial on the firmware side.
I think being able to add encoders to multiplexers would be a nice addition, but i am looking for a clear experience for the User during configuration and a clear handling by the MF connector.

I am really reluctant to make it seem like I want to push it further, I really don't have no desire for it, but... specifically focusing on the experience for the user, what could be clearer than the solution I proposed? It practically fits this requirement to a tee.
Even having a specific "encoderMultiplexer" - which I don't dislike, to be clear - would require for the user to understand a new device and why he has to define encoders in two different ways. Not a big deal, granted, but still not the clearest solution.

At the moment I am not so happy with the way how the MultiplexerDriver is a very special device. It became part of the basedevice class and has to be treated specifically by the UI. That's why I would like to think of ways to enable the support for encoders without too much of additional extra treatment. Maybe just some refactoring would help to keep things less intertwined.

The MultiplexerDriver indeed has a peculiar nature:

• It has to have allocated pins, which ought to be chosen by the user, hence it must appear in the UI.
  (It could also use preassigned pins which are silently reserved, but then the conflict must be handled if the user has already selected them for other devices;)
• it has no "output value" (ie input or output channel, or result value) available to the user, like other devices; the result of setting its control lines is only at circuital level.

The way I personally see it,, the requirement to be a peripheral driven through pins makes it sensible to implement it through a device object, which is quite fit for the purpose and does not require introducing other object types;
the lack of an "output value", ie its system-level nature, somehow destines it to be hidden in many places and managed implicitly (mostly for user convenience), but I don't see a great problem in this.
I am looking forward to our live chat where maybe we can make good progress on this recurring point of discontentment.

[elral]

I did some tests to which time the "debounce" time for encoders could be increased.
For now we are reading the inputs every 1ms, that's also what I have used in the past in other projects.

• 1ms: Very very seldom a wrong direction is detected if turning the encoder fast
• 2ms: same as for 1ms
• 3ms: seldom a wrong direction is detected if turning the encoder fast, a little bit more than 1ms/2ms, but I think acceptable
• 4ms: same as 3ms
• 5ms: turning the encoder slow is OK, but turining fast gives a lot of wrong directions
• 10ms: even slow turining doesn't work at all

So I would say targeting 2ms for reading the encoder inputs is OK, if it ends up "sometimes" to 3ms or 4ms (time delay in the main loop() for some reasons) it would be also OK.

In one of the graphic libs I saw that they implemented a "fast I/O" to speed up accessing the I/O's. As we mostly access the I/O's, we could think about to use it (or similiar). But that would mean to refactor nearly all files...

[GioCC]

...this is limited to outputs ...

I can't see the reason for this.

I meant, the "skip the check for PWM" part is something that is limited to outputs. No such thing for inputs obviously (why bother checking?). The rest of the optimizations (port+mask) is the part that is relevant for inputs.

Yes, that is true. But the pin number must not be stored anymore.

That's one of the points where I mean with not "optimized" for now.

What are you quoting here with the "Yes, that is true..." part? I can't find a reference... (Maybe something I later edited?)

I just did a test for the input and output shifter (still could be done better).
For the input shifter the time is reduced from ~50ms to ~13ms, so nearly a factor of 4. For the output shifter the time is reduced from ~74ms to 16ms, so nearly a factor of 4.6.

That's about the expected improvement... still, you have to know the pin number (port+mask) at compilation. How would you manage that with configurable pins, i.e. pins whose number is not known until runtime (or better, after config is read)?

Easiest thing I can think of is: receive pin # from config, lookup port+mask only once at config processing, store them (and there goes your RAM!), and finally use the stored values during operation.

RAM consuptiom after compilation is reported as the same (Hmhm, not what I have expecetd, should be 1byte more for each pin). >

I might venture to say that you probably don't see any additional RAM consumption because you do not store port+mask data for each pin, but use constants when calling the functions (I don't have the complete example you are testing, so I can't tell for sure).

Of course, we could assign fixed pins for SPI CLK/DataIn/DataOut lines (and also multiplexer selector lines), shared system-wide by all shift regs/multiplexers; in this case this optimization would be feasible, and almost trivial (BTW, that's how SimVim also does, for good reasons).

Even if those pins are not fixed, but left configurable - as long as they're shared by all SPI devices, the additional RAM occupation would be negligible, so still a very good option.

If one of these two options could be chosen freely, I would say that a fixed allocation would be easier on all counts, a shared+configurable allocation would IMHO be (unnecessarily) more complicated and without actual advantages.

Whatever the option, this would involve changing the MF "assign to every device its own set of pins" philosophy (and require quite a few changes in the connector GUI); however IMHO this choice would make more sense. It should probably have been considered when shift registers were first introduced.

Same for output shifters. Setting the pin could be done in one function with a given value, furthermore not to be included in each class but an own overall function,

The concept might be something like the "multiplexer driver" for multiplexer... a common handler for common pins.

Last additional thought (maybe a bit pessimistic): let's hope that the increased switching speeds won't create issues with long and bad wiring (bad comms or interference).

[GioCC]

A clarification that might be useful for whoever follows this discussion: it was somewhat implied in the above discussion that there's no need to apply this modification (storing the port+mask data) to every pin - it would be of course an advantage, but impractical.

Inputs and outputs on direct pins can be managed with the standard access functions with little penalty: what is mainly involved here are the control lines that are switched many, many more times when polling devices.
This includes shift registers (I/O and LED drivers), MAX displays, multiplexers, and possibly other devices. Not the totality of onboard pins, bit still a sizeable number for RAM requirements..

[elral]

Easiest thing I can think of is: receive pin # from config, lookup port+mask only once at config processing, store them (and there goes your RAM!), and finally use the stored values during operation.

That's exactly how I have done it. And it's not only the PWM check which is not done, it's also the port and pin calculation each time digitalRead/Write() is called.

[GioCC]

That's exactly how I have done it. And it's not only the PWM check which is not done, it's also the port and pin calculation each time digitalRead/Write() is called.

OK, that pretty much matches what I have been saying so far (and it's also the whole point of the modification...).
The important question is now all I have mentioned about the additional RAM required (+3 bytes per pin) and sharing the control lines (so this additional RAM is reduced to a tolerable minimum).

[GioCC]

Oh, and one more thing I have to correct from what I wrote the start of the discussion: now that we focus on the driver lines, it is not actually true that we are mainly interested on gains on inputs, rather tke opposite.