Skip to content

load_cell: Optional continuous tracking of loadcell measurements#7283

Open
dmbutyugin wants to merge 1 commit into
Klipper3d:masterfrom
dmbutyugin:ads131m0x
Open

load_cell: Optional continuous tracking of loadcell measurements#7283
dmbutyugin wants to merge 1 commit into
Klipper3d:masterfrom
dmbutyugin:ads131m0x

Conversation

@dmbutyugin

Copy link
Copy Markdown
Collaborator

This PR changes the default behavior of load_cell and load_cell_probe to not start tracking force after Klipper startup. This can be changed back via a configuration option, or enabled and disabled at runtime on-demand via a command. This can be particularly useful for fast-sampling loadcells as continuous sampling when not needed creates unnecessary load on the MCU and the host. And FWIW, this is an initial proposal and I'm open to suggestions on the command and configuration option names and such.

@garethky FYI

Comment thread docs/G-Codes.md Outdated
is calibrated a force in grams is also reported.

### LOAD_CELL_TRACK_FORCE
`LOAD_CELL_READ [LOAD_CELL=<config_name>] [TRACKING=<START|STOP>]`:

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LOAD_CELL_TRACK_FORCE

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right, my bad, copy-paste from the wrong place.

Signed-off-by: Dmitry Butyugin <dmbutyugin@google.com>
@garethky

Copy link
Copy Markdown
Contributor

My preference is to have it on by default. This is more like a temperature sensor because it has a status. Having it off just adds confusing hoops for the user to jump through to get it working.

Positives:

One use-case I can think of for this is if you have have multiple toolhead probes and you want just 1 on at a time. So this probably has to happen. (klipper cant do that, yet)

Keeping the sensor on over long running set of tasks (like a print start or bed mesh) is beneficial. Programming the sensor registers, starting measurements and sensor settling time all... take time. (right now we ignore the settling time because the sensors are always running at startup) You wouldn't want to wait for that for every probe.

So, for those two reasons, I endorse the idea of a command to keep thing on and turn them off.

Not so good:

It doesn't look like using any of the load cell gcodes (LOAD_CELL_*) or starting a probe would activate the load cell automatically. The existing contract with the other off-by-default sensors is that they auto start on use (there is no separate imu-tracking start/stop). Particularly the probe is scary because that could crash a machine. (Or it will violate the watchdog on the MCU and cause a shutdown?) Either way this looks like a foot gun that you will inflict on all users.

It looks like all clients are disconnected when tracking stops. That could cause the front end to go into a re-connect loop or require some complicated reconnect logic for internal modules that want to monitor force. I would send a tracking started/tracking stopped message instead and allow the subscription to remain. If a web client connects and the sensor isn't tracking, just send it the stopped message. The message pump is event driven so this shouldn't add any overhead in the stopped state.

Alternatives:

What I would do:

  • Any command that needs sensor data activates the sensor and deactivates it when it competes, just like the existing sensors.
  • probe.start_probe_session starts tracking and keeps tracking on for the entire session (solves for bed_mesh, QGL etc.)
  • LOAD_CELL_TRACK_FORCE enables and disables persistent tracking of a sensor
  • clients stay connected and get tracking start/stop messages

Lower the sample rate:
For PA and probing to co-exist, its probably going to be necessary to change the sample rate on the fly for the task. If you can drop the sample rate at runtime you can keep the sensor on for things like nozzle clog or spaghetti detection. So you could have: LOAD_CELL_SAMPLE_RATE RATE=50
And then do 50sps for printing, 500sps for probing and 4000sps for a PA calibration.
This is probably what I'm going to do in Kalico first. When we get to multi-tool systems I'll have to implement LOAD_CELL_TRACK_FORCE or some sort of multiplexing setup for tools that makes it automatically switch.

@KevinOConnor

Copy link
Copy Markdown
Collaborator

Thanks. For what it is worth, I don't think a new track_force config option adds much. If I understand correctly, today there are force_g, min_force_g, and max_force_g exported status values, and the goal is to make these optional because it is not ideal to always run the sensor at a full rate. The proposal to add a LOAD_CELL_TRACK_FORCE command to enable those three force_g exports seems like a good choice to me. However, I think the new track_force config option adds user facing complexity without much gain - I think we should consider converting to the command without a new config option.

As Gareth mentions, the sensor reading would need to start automatically on a probe_session_start() and similar commands (as is done for the other sensors). Also, the sensor should not stop reading values until there are no clients of the data (as is also done for the other sensors).

Separately, as a high-level observation, we may want to look at replacing load_cell.ApiClientHelper() with the common bulk_sensor.BatchBulkHelper(). And also look at refactoring probe_eddy_current.EddyGatherSamples() and load_cell.LoadCellSampleCollector() into a common helper class (eg, bulk_sensor.GatherSamplesHelper()). I can look at doing this if that helps.

Cheers,
-Kevin

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants