BUG: Multi use pins / shared pins don't work if aliased

There’s an inconsistency in how board_pins aliases work compared to regular pin names. In short, this works:

[tmc2209 stepper_x]
uart_pin: PC11

[tmc2209 stepper_y]
uart_pin: PC11

But this doesn’t.

[board_pins test]
aliases:
    x_uart_pin=PC11,
    y_uart_pin=PC11

[tmc2209 stepper_x]
uart_pin: x_uart_pin

[tmc2209 stepper_y]
uart_pin: y_uart_pin

[duplicate_pin_override] doesn’t work for board_pins aliases either. The PinResolver doesn’t know anything about pin params or multi use pins.

The alias function is implemented pretty rudimentary only. There are numerous posts here that discuss similar topics:

I am aware, and there hasn’t been a sufficient answer before either. This is weird, confusing, and resembles a bug, more than something intentional. It’s not explained in the docs, and no sufficient workaround is offered. Furthermore, it seems like the primary reason nothing has been done about this is the following quote from kevin:

I understand why you are looking for it. It’s not clear to me that you can reach your goals in general. Many boards are fundamentally different (eg, Duet2 boards use inverse heater pins, some boards use 2200 ohm pullups, etc.).

Long before the railcore devs attempted their config abstraction, i’d done so for multiple printers and hardware with RatOS, which is currently counting 25 individual boards and thousands of users. Goal successfully reached. It’s starting to be really annoying and require ugly hacks to work around this (very few boards require it, but it is a thing), and now i’m bringing it up again after github issues have basically been shut down. I’m willing to submit a PR to fix it. It looks a lot like a bug in its current form, so if intentional, there’s some documentation to do, and a sufficient workaround needs to be presented, imo.

Best thing to do is to check with @koconnor. If you read the other threads his enthusiasm to change this seemed, let’s say, “limited”.

For me personally the aliases could go away altogether. It just obfuscates the configuration and the only goal that RatOS reached was to make the cfg completely incomprehensible and no longer modifiable for the average user. But this is just my personal opinion.

1 Like

Best thing to do is to check with @koconnor.

Hence the post.

For me personally the aliases could go away altogether.

Honestly, that would probably be better than the current situation. It would’ve prevented countless new users from learning klipper though. Having aliases allow for abstraction, which is much needed to improve the klipper user experience. I’ve heard kevin say that having an interface to configure one’s printer was something to strive for, without proper working aliases that’s going to be infinitely harder, or you’ll basically end up with a text editor with more steps. Setting up a printer through an easy to use interface is the end goal of RatOS and we’re getting closer with each release. I’m solving a problem, i’m trying to get klipper into the hands of more people, people who aren’t software engineers or sysadmins or otherwise PC superusers, you may not like my solution, but at least i hope we can agree that it’s a worthwhile goal.

It just obfuscates the configuration and the only goal that RatOS reached was to make the cfg completely incomprehensible and no longer modifiable for the average user. But this is just my personal opinion.

Have you used it since you think yourself qualified to judge it, unconstructively I might add?

I copied your example board_pins alias into my srk-mini-e3 printer config and it works for me.

If there’s an error you should attach the full Klipper log file from the event (while running the pristine Klipper code).

-Kevin

You’re right, my sample code was incorrect.

[board_pins test]
aliases:
    x_uart_pin=PC11,
    y_uart_pin=PC11

[tmc2209 stepper_x]
uart_pin: x_uart_pin

[tmc2209 stepper_y]
uart_pin: y_uart_pin

Should do it.

There are no modifications to klipper in RatOS. There are a few extras though (gcode_shell_command by Arksine and linear_movement_analysis), but it’s the same without them.

Config error
Traceback (most recent call last):
  File "/home/pi/klipper/klippy/klippy.py", line 180, in _connect
    cb()
  File "/home/pi/klipper/klippy/mcu.py", line 732, in _connect
    self._send_config(None)
  File "/home/pi/klipper/klippy/mcu.py", line 671, in _send_config
    cmdlist[i] = pin_resolver.update_command(cmd)
  File "/home/pi/klipper/klippy/pins.py", line 53, in update_command
    return re_pin.sub(pin_fixup, cmd)
  File "/home/pi/klipper/klippy/pins.py", line 48, in pin_fixup
    name, self.active_pins[pin_id]))
pins.error: pin y_uart_pin is an alias for x_uart_pin

Okay, but an error is the intended response in that case.

It’s not possible to use the same pin for two different modules, unless those modules have been explicitly coded to handle the complex synchronization necessary to “share” a pin. I know that is confusing for users, because they think “it’s just a config entry, why does the software keep complaining”. If one “takes a step back” though, it should be clear that two different modules can’t both “just use the same pin” - it would be chaos. Different pin updates would get written out of sequence, neither module would know that something else changed the pin, minimum pin pulse durations wouldn’t be honoured, etc.

When the “tmc uart” code detects that there are multiple drivers declared with the same pin name, it instantiates a series of internal classes to synchronize access to those drivers. It also enables additional config entries (for uart_address and select_pins muxing).

This uart sharing isn’t enabled if the code doesn’t know that the pin is shared. Fully resolving the pin names doesn’t occur until after contacting the micro-controller, while the config is fully read prior to connecting to any micro-controllers. (It would be quite difficult to connect to the micro-controllers without reading the config specifying what micro-controllers are present.)

Thus, “shared pins that don’t look like shared pins” is not a simple change.

Beyond the technical challenges there is the issue of support and debugging. Fundamentally, the code does something notably different on a “shared pin” and obscuring that a pin is shared in the config seems like a misfeature to me.

I understand you would like to handle boards that are fundamentally different as if they are the same. I am unconvinced that it is practical or desirable to do so.

Cheers,
-Kevin

1 Like

I would also add that I’m not sure what the gain of x_uart_pin and y_uart_pin aliases that point to the same pin is. Ultimately one would still need to declare different uart_address fields anyway. So, these boards can’t be treated as similar regardless of pin aliases.

Cheers,
-Kevin

Thank you for taking the time to explain. I’m with you, i understand the problems you’re listing, I just don’t understand why the alias lookup has to interfere with any of it, since it works fine without aliasing.

No alias) Pin PC11 → We’re already using that → Special behavior
Expected alias behavior) Resolve the alias → Pin PC11 → We’re already using that → Special behavior
Current alias behavior) Resolve the alias → Nope PC11 is already defined as another alias → Dead.

You don’t need to contact the micro controller to resolve an alias, it’s a simple map, right? Why isn’t that a step before the pin validation that happens anyway?

Thus, “shared pins that don’t look like shared pins” is not a simple change.

In my mind (obviously i have none of the insight you have, I understand if you don’t want to waste time entertaining mr. smooth brain here, so if i’m just being dumb, i accept that), the aliases should be resolved to their pin names as a separate step. That way you have consistent behavior, and only need validation in one place, instead of having to implement validation twice (or not at all, as is the case currently, they just disallow multiple aliases pointing to the same pin when they’re used in the config, you can define them just fine). What’s the problem with that approach?

Beyond the technical challenges there is the issue of support and debugging. Fundamentally, the code does something notably different on a “shared pin” and obscuring that a pin is shared in the config seems like a misfeature to me.

If the pin aliases were resolved to their pin names as a step in constructing the loaded configuration (the one that appears in the log), that would be less of an issue.

I understand you would like to handle boards that are fundamentally different as if they are the same. I am unconvinced that it is practical or desirable to do so.

Currently impractical and messy, but not impossible (we have a working config). Very much desirable, i’ve been hesitant to add the SKR e3 mini and duet boards, because they’re so messy to abstract (and Duets because of the lack of flashing that can be automated), because the tools we’re given do not allow it. I’ve received countless requests, and now that one came along with a user contributed Voron V0.1 config, i had to give in. Currently we’re working with a quirks file include, and it physically hurts me.

I would also add that I’m not sure what the gain of x_uart_pin and y_uart_pin aliases that point to the same pin is. Ultimately one would still need to declare different uart_address fields anyway. So, these boards can’t be treated as similar regardless of pin aliases.

I think showing is easier than telling in this case.
SKR 3 config
SKR MINI E3 V3.0 config

Since the mini e3 has soldered 2209’s, the tmc2209 sections can be defined in the board config. Right now i need a “quirks file” later in the configuration to set the uart_pins manually to PC11, after all overrides are done. It’s … really messy. The uart address and tx_pin can be set to their defaults by overriding includes, for example in case of toolboard support so that’s not an issue, if it wasn’t for the fact that I can’t reuse e_uart_pin for tx_pin in [tmc2209 extruder] because of the current alias limitation.

The simple answer is, “it wasn’t implemented that way”. Ultimately, the micro-controller is not sent a pin name, it is sent a number. The pin names and their corresponding numbers are stored on the micro-controller. In that regard, the “pin aliases” are treated similarly to the handling of nominal “pin names”. The current implementation resolves the aliases during the pin name resolution process; the check for mixing of aliases is also implemented at that point. If you look closely you’ll see that the error cited above is raised after the micro-controllers are contacted. The error is not to say “you’re doing things the wrong way”, but to inform the user that “the software is definitely not going to work correctly with that config”.

Other implementations are possible. They would likely have their own caveats. Order of config sections in the config would be more important. A different set of error checks would likely be needed.

Cheers,
-Kevin

1 Like

Ultimately, the micro-controller is not sent a pin name, it is sent a number.

I figured. I can see now how resolving the aliases at the same time was the logical thing to do.

The error is not to say “you’re doing things the wrong way”, but to inform the user that “the software is definitely not going to work correctly with that config”.

Except it is going to work correctly (simply replacing the aliases with the pin name makes it work again), that’s what’s bothering me.

Other implementations are possible. They would likely have their own caveats. Order of config sections in the config would be more important. A different set of error checks would likely be needed.

I’m going to mess around a bit and break things, see where it gets me. Thanks for taking the time!

It does not work correctly! It will lead to the “chaos” I cited above. Incorrect timing of pin updates, random run-time crashes, odd trinamic driver error reports, and other problems. The system will be unstable in a way that is not apparent in any short term test. Beware.

-Kevin

Edit: Just to be clear, going back to a supported config is fine, but disabling that check in the low-level code would lead to random faults.

It does not work correctly! It will lead to the “chaos” I cited above. Incorrect timing of pin updates, random run-time crashes, odd trinamic driver error reports, and other problems. The system will be unstable in a way that is not apparent in any short term test. Beware.

I understand that, but seen from a user perspective i’m telling it to use the exact same pin, x_uart_pin = y_uart_pin = PC11. Just entering PC11 multiple times works, more than one alias for PC11 does not. I realize that somewhere, PC11 (or another alias) gets flagged as a “shared pin”, and only one pin alias (even though they would resolve to the same microcontroller number in the end) can be a shared pin for that particular section at any given point in time. At least that’s how I understood your explanation. Am i correct so far?