Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Load cell gram scale #6729

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

garethky
Copy link
Contributor

@garethky garethky commented Nov 5, 2024

This PR adds the features needed to make a load cell actually work as a gram scale. This PR will let you weight filament or measure a force in grams. (no homing or probing yet!)

  • Convert raw sensor counts to grams and make this force data available via unix socket and printer object status
  • Track sensor calibration state and allow sensors to go from un-calibrated to calibrated at runtime
  • Add basic GCodes for tearing and reading the load cell
  • Add a guided calibration tool so users can calibrate a load cell with a known mass or measured force.
  • Add diagnostic GCode to check the health and performance of the load cell and sensor

This code has been tested by several people in the community and some last minute bug fixes and changes were landed:

  • The reverse option allows the polarity of the force readings to be inverted. You can have your probing collision graphs in either orientation now.
  • reference_tare_counts is written to config and read as an integer. link
  • Fixed crash when guided calibration tool being entered twice
  • Fixed crash when CALIBRATE was used before TARE

A lot of work (months) and bug fixing went into parts of this, particularly LOAD_CELL_DIAGNOSTIC and the LoadCellSampleCollector. These bits have to work when the sensor is buggy and still produce usable output. LoadCellSampleCollector also underpins later work for probing.

numpy is required for this PR. We could change that. But PR's for probing will absolutely need it. If you want to merge [load_cell] and [load_cell_probe] then it seems there is no point in trying to keep it out of this PR.

@garethky
Copy link
Contributor Author

garethky commented Nov 5, 2024

The tests failed because of the numpy dependency. I guess I could move the check out of __init__ to get them to pass.

@JamesH1978
Copy link
Collaborator

such a pity numpy is so bloaty and time consuming to just add it to requirements

@Sineos
Copy link
Collaborator

Sineos commented Nov 6, 2024

such a pity numpy is so bloaty and time consuming to just add it to requirements

I'm not sure that this has a real world impact. I guess most, or at least a significant portion also use input shaping and have it installed anyway.
Might be even better to add it to the requirements and call for a version <1.26 to avoid clashing with input shaping.

@JamesH1978
Copy link
Collaborator

i think its the sheer time to compile numpy is the issue if the host is sub par, it can take hours on some

@garethky
Copy link
Contributor Author

garethky commented Nov 6, 2024

I would love to get on my "just require numpy" soap box (Binaries are installed automatically for 32bit, you don't need to compiling it. Its shockingly fast vs plain python. "Your job as a Python programmer is to write C") but... I want to ship this and be 'done' with the project. So what do I have to do?

I can push the numpy require check back to the point where its clear that you are configuring a probe. Most of what I'm using it for in this PR (average, min, max, unique) is relatively easy to replace or remove.

For probing, I need linalg.lstsq. There are some optimizations I discussed with @KevinOConnor to replace the bulk of those calls, but I still have to make several calls to plain least squares. numpy/c is so much faster than Python that any sort of optimizations that we do might be washed away because they aren't in c (plus the code is very complicated). There is also, very very likely, a numpy/vectorized version of what we want to do that will crush the pure Python version (it has loops, its probably 50x slower than numpy). If this code is slow, it causes noticeable pauses.

@Sineos
Copy link
Collaborator

Sineos commented Nov 6, 2024

Is there any way to provide / use a prebuilt v1.26 wheel?


This document describes Klipper's support for load cells and load cell based
probes.

Copy link
Contributor

Choose a reason for hiding this comment

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

not knowing what a load cell does/means for a 3d printer, could you add a sentence or two describing it's usage/benefits for 3d printers/klipper?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I made some updates to the documentation including a little bit more exposition at the top and a section about how to read the load cell gram force

@garethky garethky force-pushed the pr-load-cell-gram-scale branch 2 times, most recently from a3f6aa5 to dde606a Compare November 11, 2024 03:08
@garethky
Copy link
Contributor Author

Is there any way to provide / use a prebuilt v1.26 wheel?

For users: if you use Python 3.0 on a 32 bit RPI OS then PiWheels will just install a binary. That's the default behavior. You have to kinda work hard to not get that to happen. i.e. willfully use Python 2 or pick a 64 bit OS.

For the git build system: I'm not sure how that works.

@Sineos
Copy link
Collaborator

Sineos commented Nov 11, 2024

For users: if you use Python 3.0 on a 32 bit RPI OS then PiWheels will just install a binary.

PyPi has some arm64 wheels as well

For the git build system: I'm not sure how that works.

There are examples of project who built the necessary wheels as CI actions. E.g. https://github.com/matrix-org/synapse/pull/14212/files

Maybe it would be an option to built the needed wheels (Python 2 and Python3?) as Klipper CI and the install scripts (or KIAUH) could automatically draw upon them.

@garethky
Copy link
Contributor Author

This was updated to remove the numpy dependency

# Load Cells

This document describes Klipper's support for load cells. Basic load cell
functionality can be used to read force data and to weigh things like filament.

Choose a reason for hiding this comment

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

Consider adding a note that weighing things is only possible on printers with the load cells in the bed, and not those with the cell in the extruder.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't want to get into applications because there are many. You don't even have to put it in the printer! Several people want to use this functionality to weigh filament spools mounted outside the printer. E.g. https://klipper.discourse.group/t/filament-spool-scale-hx711/19800/

  • You could check before a print if there is enough filament on the spool based on the print weight
  • Track filament used by weight.
  • Track drying if the filament by weight lost
    In the future this may be in an application that is just a dry box and not even a printer.
    One community member is even working on runout detection in a resin printer.

Copy link
Collaborator

@KevinOConnor KevinOConnor left a comment

Choose a reason for hiding this comment

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

Thanks. Sorry for the delay in responding.

In general it seems fine to me. I have a few comments and questions - see below.

Cheers,
-Kevin

Comment on lines +399 to +402
### load_cell/dump_force

This endpoint is used to subscribe to force data produced by a load_cell.
Using this endpoint may increase Klipper's system load.
Copy link
Collaborator

Choose a reason for hiding this comment

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

It seems odd that there is an additional endpoint added for this information. I would have thought the information could be added to the existing hx71x/dump_hx71x endpoint (or that endpoint replaced with this one).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Well there are 2 ADC endpoints hx71x/dump_hx71x and ads1220/dump_ads1220.

My goal with this was to try to keep everything "load cell" related out of the ADC code. The load_cell is acting as a decorator to the raw ADC data stream. The raw stream is still useful as a diagnostic tool (or at least it was during development).

This goes with your other question about extending/updating the socket code. What we decide here is going to determine what to do about that comment.

At a high level, what are you thinking about here?

  • Is the performance penalty of the duplicate streams a concern?
  • Is the duplication of the raw counts data in 2 streams confusing?
  • Everything is one stream is desired but then the ADC stream is redundant/confusing?

Copy link
Collaborator

@KevinOConnor KevinOConnor Dec 1, 2024

Choose a reason for hiding this comment

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

A bit of all three. Mostly concerned with maintaining two different interfaces that provide redundant capabilities, and the resulting developer confusion on why there are two redundant interfaces.

Perhaps take a look at how ldc1612.py takes a calibration object and calls self.calibration.apply_calibration(samples), which then allows probe_eddy_current.py to populate a "Z" field in the samples.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I look at the ldc1612 code and I think the other option is a better fit for this use case. The "that endpoint replaced with this one" idea.

First, I think its more developer friendly to have the endpoint be named according to the load cell in the config and not the sensor implementation. That's a leaky abstraction. That way a front end doesn't need to look up the sensor type in the config and try to build the endpoint URI. The URI can be built from the load cell name alone and its always consistent. Force data comes from a thing that is about force. That feels consistent and logical. (You could argue that LDC1212 is only about Z so that's not necessarily inconsistent)

Second but less important: the 'B' channel on the HX717 is used to measure an RTD and a hall effect filament sensor in Prusa's new machines. I think it would make supporting that kind of thing less awkward later on if we deleted the endpoint from the sensors. Objects that consume the data (LoadCell, Temperature etc.) can transform and publish it (or not) as required.

I'll work on this first.

Copy link

Choose a reason for hiding this comment

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

less important: the 'B' channel on the HX717 is used to measure an RTD and a hall effect filament sensor in Prusa's new machines

Hey, that is important! ;)

I've currently got a very rough proof of concept implementation for the filament sensor, based on your load-cell-probe-community-testing branch -- ie, using the WebhooksHelper/WebhooksTransformer in this PR. Probably the main feature that would be useful for my code is being able to subscribe to a particular channel only. At the moment my code has 3 or 4 places all receiving the whole data stream and doing their own filtering on it.

In day-to-day usage the raw counts data is not that useful, especially to the user -- dropping the raw sensor API endpoint seems fine. However, if a load cell ADC were to be reused by existing modules that currently just rely on regular ADC pins (gcode_button, hall_filament_width_sensor), the user will need access to the raw counts or the normalized 0..1 value somehow during the setup process in order to calibrate their new gadgets, since they won't necessarily have specific commands like in this PR. For the filament sensors it doesn't have to be the existing endpoint, though. The values change dramatically enough that getting even just one data point each with filament inserted and not inserted would allow for a reliable configuration.

Comment on lines +408 to +410
`{"id": 123,"result":{"header":["time", "force (g)", "counts", "tare_counts"]}}`
and might later produce asynchronous messages such as:
`{"params":{"data":[[3292.432935, 40.65, 562534, -234467]]}}`
Copy link
Collaborator

Choose a reason for hiding this comment

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

Just out of curiosity, does "tare_counts" change with each sample?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, it only changes when the tare command is executed.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Okay - seems a little odd to repeat this value for every sample. Maybe just export the information via get_status()?

Copy link

Choose a reason for hiding this comment

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

The load cell probe implementation currently re-tares before homing, so API consumers may find this valuable over time?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Because a Tare can happen in the middle of a batch of results you wouldn't know which samples used what tare value. A UI would not correctly reflect whats really going on. And as pointed out, tare can happen frequently while probing.

Comment on lines +4668 to +4772
#reverse:
# Reverses the polarity of the load cell. This is a boolean value, the
# default is False.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Out of curiosity, is "reverse" effectively the same as a negative "counts_per_gram"?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Basically. This lets you do what you wanted, where if the force graph is naturally negative you can reverse its polarity.

Comment on lines +708 to +755
### LOAD_CELL_DIAGNOSTIC
`LOAD_CELL_DIAGNOSTIC [LOAD_CELL=<config_name>]`: This command collects 10
seconds of load cell data and reports statistics that can help you verify proper
operation of the load cell. This command can be run on both calibrated and
uncalibrated load cells.

### CALIBRATE_LOAD_CELL
`CALIBRATE_LOAD_CELL [LOAD_CELL=<config_name>]`: Start the guided calibration
utility. Calibration is a 3 step process:
1. First you remove all load from the load cell and run the `TARE` command
1. Next you apply a known load to the load cell and run the
`CALIBRATE GRAMS=nnn` command
1. Finally use the `ACCEPT` command to save the results

You can cancel the calibration process at any time with `ABORT`.

### TARE_LOAD_CELL
`TARE_LOAD_CELL [LOAD_CELL=<config_name>]`: This works just like the tare button
on digital scale. It sets the current raw reading of the load cell to be the
zero point reference value. The response is the percentage of the sensors range
that was read and the raw value in counts.

### READ_LOAD_CELL load_cell="name"
`READ_LOAD_CELL [LOAD_CELL=<config_name>]`:
This command takes a reading from the load cell. The response is the percentage
of the sensors range that was read and the raw value in counts. If the load cell
is calibrated a force in grams is also reported.
Copy link
Collaborator

Choose a reason for hiding this comment

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

As a general comment, it's generally preferable to implement less commands. For example, could this be reduced to just two commands: LOAD_CELL_READ (for reading and diagnostics) and LOAD_CELL_CALIBRATE (for calibrating and changing the tare).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok, I can make those changes.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Does the gcode parser work if you dont assign a value to things. Like, could this work:

LOAD_CELL_READ
LOAD_CELL_READ TARE
LOAD_CELL_READ DIAGNOSTIC

@@ -0,0 +1,102 @@
# Load Cells
Copy link
Collaborator

Choose a reason for hiding this comment

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

If a new document is added then it's also necessary to update docs/Overview.md and docs/_klipper3d/mkdocs.yml

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Will do

# Adapter for WebhooksHelper that transforms the response using a function
# Anything that implements the add_client contract can be a message source
# Outputs to its own clients
class WebhooksTransformer(WebhooksHelper):
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can you elaborate on what this does (as well as WebhooksHelper)?

At first glance, it seems a little odd to make a slightly different interface for just loadcell. If the BatchWebhooksClient code isn't flexible enough, maybe we should expand the webhooks interface and refactor the other users (like ldc1612 and accelerometers).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This goes back to your first comment, lets resolve what we want for a data stream structure. If this is still required I can do a refactor.

Comment on lines +427 to +430
self.printer.send_event("load_cell:calibrate", self)
if self.is_tared():
self.printer.send_event("load_cell:tare", self)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can you elaborate on what these events will be used for?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The probing code uses these events to get notified about the state of the gram scale. When the user interactively calibrates the load_cell for the first time it goes from uncalibrated -> calibrated. That state transition allows probing to work without a restart. Before that it will throw an error that you are not allowed to probe or home with a load cell that isn't calibrated.

I still have the probing code in a separate file because I believe this chunk of functionality is a nice cleanly separate unit. My intention is to pitch you on keeping them split up. These events are one part of the decoupling that makes that work.

Copy link

Thank you for your contribution to Klipper. Unfortunately, a reviewer has not assigned themselves to this GitHub Pull Request. All Pull Requests are reviewed before merging, and a reviewer will need to volunteer. Further information is available at: https://www.klipper3d.org/CONTRIBUTING.html

There are some steps that you can take now:

  1. Perform a self-review of your Pull Request by following the steps at: https://www.klipper3d.org/CONTRIBUTING.html#what-to-expect-in-a-review
    If you have completed a self-review, be sure to state the results of that self-review explicitly in the Pull Request comments. A reviewer is more likely to participate if the bulk of a review has already been completed.
  2. Consider opening a topic on the Klipper Discourse server to discuss this work. The Discourse server is a good place to discuss development ideas and to engage users interested in testing. Reviewers are more likely to prioritize Pull Requests with an active community of users.
  3. Consider helping out reviewers by reviewing other Klipper Pull Requests. Taking the time to perform a careful and detailed review of others work is appreciated. Regular contributors are more likely to prioritize the contributions of other regular contributors.

Unfortunately, if a reviewer does not assign themselves to this GitHub Pull Request then it will be automatically closed. If this happens, then it is a good idea to move further discussion to the Klipper Discourse server. Reviewers can reach out on that forum to let you know if they are interested and when they are available.

Best regards,
~ Your friendly GitIssueBot

PS: I'm just an automated script, not a human being.

@garethky garethky force-pushed the pr-load-cell-gram-scale branch 2 times, most recently from 52e6c57 to a60add0 Compare February 25, 2025 00:07
* Convert sensor counts to grams and make this available via unix socket and object status
* Basic GCodes for tearing and reading the load cell
* Guided Calibration
* Diagnostic gcode to check the health of the load cell

Signed-off-by: Gareth Farrington <[email protected]>
* Add API server load_cell/dump_force endpoint
* Update [load_cell] config with calibration fields
* Add G-Code commands for working with load cells
* Add status reference for load_cell objects

Signed-off-by: Gareth Farrington <[email protected]>
@garethky garethky force-pushed the pr-load-cell-gram-scale branch from a60add0 to d47eb77 Compare February 25, 2025 01:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants