DRAFT: query for entry sensor status

[LongLiveCHIEF]

Adds gcode command cmd_TR_QUERY_LANE_ENTRY_SENSORS, along with corresponding documentation updates.

[LongLiveCHIEF]

Since I'm new to Python, I wanted to get this out there in front of you as a draft while I wrap up some testing, so that you can give me any feedback you might have on my approach to the features you wanted for this.

Hoping to do testing and validation of this tomorrow if all goes as planned.

[rsghosh]

Thanks for working on this! This looks very good overall, but it looks like right now the TR_QUERY_LANE_ENTRY_SENSORS gcode command is checking whether trigger response is active for each sensor (i.e. whether or not the trigger callback will be called if the sensor transitions to the triggered state) rather than the current state of the sensor. Similarly, the lane_entry_sensors status is reporting whether both trigger and untrigger responses are active for the sensor. I think this should be a relatively quick fix though.

To report the last state of the sensor instead, I think we'll need to add a "state" variable to the TradRackFilSensor class and set that inside sensor_callback() to the state argument that is passed in whenever that function runs (since the class currently does not track the last sensor state). And then we could have something similar to _get_lane_entry_sensors_active() in the TradRack class that will check the state variable of each sensor rather than whether trigger and untrigger responses are active.

The document changes look good, I only have a few minor comments:

• In the table of contents for G-Codes.md, TR_QUERY_LANE_ENTRY_SENSORS is missing a link
• I noticed you removed all the backslashes before the underscores in the table of content items in G-Codes.md and Config_Reference.md. I've just been using the automatic table of contents creation/update commands from the VSCode "Markdown All in One" extension for handling the table of contents, so I haven't really thought about this before; I think the backslashes were meant to ensure that the underscores are interpreted as normal text rather than symbols for making surrounded text italicized, but it seems like GitHub is correctly displaying the text even without the backslashes on your branch of the repo. I'm not really sure which is the best approach here, but currently in G-Codes.md TR_QUERY_LANE_ENTRY_SENSORS has backslashes while the rest don't, so whichever way we go I think we should make all of them match.
• (once the code is updated to check the last sensor state rather than trigger/untrigger response being active, the description for the lane_entry_sensors status will need to be updated)

[LongLiveCHIEF]

gcode command is checking whether trigger response is active for each sensor (i.e. whether or not the trigger callback will be called if the sensor transitions to the triggered state) rather than the current state of the sensor

Yep, picked up on this last night when I finally got a chance to start actually testing the code.

TR_QUERY_LANE_ENTRY_SENSORS has backslashes while the rest don't, so whichever way we go I think we should make all of them match.

I use M AIO as well... I will add a .vscode/recommedations.json and a settings.json to codify it. I didn't catch that it was making that change. My profiles get complicated because I need to link my github to my work, but I tend to have extensions in my profile that I can't use at work (spotify/discord presence, etc...), so sometimes it does something unexpected on save. Sorry!

[LongLiveCHIEF]

My inclination is to follow the existing convention. No reason to pollute the scope of this PR.

[LongLiveCHIEF]

To report the last state of the sensor instead, I think we'll need to add a "state" variable to the TradRackFilSensor class and set that inside sensor_callback() to the state argument that is passed in whenever that function runs (since the class currently does not track the last sensor state). And then we could have something similar to _get_lane_entry_sensors_active() in the TradRack class that will check the state variable of each sensor rather than whether trigger and untrigger responses are active.

I like this plan. I had been thinking along similar lines this morning before I checked for your feedback, so I think we're on the same page on all fronts.

[LongLiveCHIEF]

I'm looking over the functionality of the TradRackFilSensor. Am I right in seeing that since it is registered asa button, it acts as a sort of virtual pin, such that you can't directly read the digital signal of the registered pin, and have to trust in your callback logic to manage the pin state correctly? (klippy reference docs aren't great on the level of detail, and there's not even a lot of comments in module code, so I'm guessing each new module developer has to basically suss everything out on their own?)

My first thought is to just import the pins module, and ignore the sensor_callback entirely in favor of a digital pin read on the assigned button pin, that can then be called from the main class.

[LongLiveCHIEF]

disregard, I hadn't yet connected how the class was also making use of pins and endstops objects.

[rsghosh]

I'm looking over the functionality of the TradRackFilSensor. Am I right in seeing that since it is registered asa button, it acts as a sort of virtual pin, such that you can't directly read the digital signal of the registered pin, and have to trust in your callback logic to manage the pin state correctly? (klippy reference docs aren't great on the level of detail, and there's not even a lot of comments in module code, so I'm guessing each new module developer has to basically suss everything out on their own?)

I believe so. I couldn't really find documentation either so I looked at similar modules such as filament_switch_sensor. I figured for the lane entry sensors, we only really need to respond to state changes and we can store the last state internally if we need, so registering the pin as a button and just needing to write a callback function seemed convenient. The button callback also runs when klipper initially starts without the switch needing to change state, so we will always get the starting state (I tested this for Belay just to be sure since it relies on knowing the initial state).

[LongLiveCHIEF]

The button callback also runs when klipper initially starts without the switch needing to change state, so we will always get the starting state (I tested this for Belay just to be sure since it relies on knowing the initial state

I've been doing a code deep dive on all the klippy modules, and yeah, pins and buttons are available even before the ready event, so you can immediately get state from them. Digital Reads are just weird in that they're the only IO that doesn't really have a direct MCU command, and if I had to guess it's because it's the most common functionality, so the various uses of digital read all have their own interfaces (endstop, stepper pins, and other things that have their own reactor thread).

Looks like all of an mcu's buttons get their own reactor thread, and they're all registered at once as non-iterrupts. During runtime, the "state" change callback execution is also asynchronous, so there isn't really a real-time digital read, but there is an available debouncing that does physical/virtual state tracking for you, so I'm wondering if the "active" state tracking implemented currently is redundant, and also if we needed to register the callback for these on the global reactor, since they're already on a reactor of their own when they're setup?

[LongLiveCHIEF]

since they're already on a reactor of their own when they're setup?

n/m, we're using self.reactor.register_callbacks so we're not creating a new reactor thread.