-
Notifications
You must be signed in to change notification settings - Fork 237
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
Add new driver for HX711 load cell ADC #1251
base: public
Are you sure you want to change the base?
Conversation
this.#hx711c = new HX711c(options.sensor.clk, options.sensor.din, options.gain || 1) | ||
this.#hx711c.read() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
configure() can be called more than once. If it is, the first HX711c instance will be replaced with another. But, there's also no guarantee that options.sensor
will be present when configure
is called, since that property is for the constructor. The simplest solution is to move these two lines to the constructor and leave configure
empty.
readable() { | ||
//return this.#din.read() == 0 | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this function necessary? It isn't in the standard. The usual behavior is to have sample()
return undefined
if no reading is available. (It looks like this is what you implemented in your native read
function already)
} | ||
|
||
void xs_HX711_destructor(void *hostData) { | ||
hx711_data *data = hostData; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The destructor can be called with hostData
set to NULL
. This happens when the virtual machine is deleted. It is rare. ;) It can also happen if an exception occurs in the constructor before the host data is set. So, the best practice is to always check for NULL
#include "modGPIO.h" | ||
#include "mc.xs.h" // for xsID_* constants | ||
|
||
#define xsmcVar(x) xsVar(x) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
unused. what was the idea?
if (data == NULL) xsUnknownError("can't allocate data"); | ||
data->gain = xsmcToInteger(xsArg(2)); | ||
if (modGPIOInit(&data->clk, NULL, clk_pin, kModGPIOOutput)) | ||
xsUnknownError("can't init clk pin"); | ||
modGPIOWrite(&data->clk, 0); | ||
if (modGPIOInit(&data->din, NULL, din_pin, kModGPIOInput)) | ||
xsUnknownError("can't init dat pin"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If any of these throw, data
is orphaned (even the call to xsmcToInteger
can call in type coercion cases). Error clean-up isn't pretty, but here's what I'd probably do here:
int clk_pin = xsmcToInteger(xsArg(0));
int din_pin = xsmcToInteger(xsArg(1));
hx711_data d;
d.gain = xsmcToInteger(xsArg(2));
if (modGPIOInit(&d.clk, NULL, clk_pin, kModGPIOOutput))
xsUnknownError("can't init clk pin");
if (modGPIOInit(&d.din, NULL, din_pin, kModGPIOOutput)) {
modGPIOUninit(&d.clk);
xsUnknownError("can't init din pin");
}
hx711_data *data = c_malloc(sizeof(hx711_data));
if (data == NULL) {
modGPIOUninit(&d.clk);
modGPIOUninit(&d.din);
xsUnknownError("can't allocate data");
}
data = d;
} hx711_data; | ||
|
||
void xs_HX711_init(xsMachine *the) { | ||
if (xsmcArgc != 3) xsUnknownError("invalid arguments"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This check isn't strictly necessary. Using xsArg on an argument that doesn't exist will throw. All this guarantees is that there aren't more than 3 arguments. No harm in the check, but it is common for JavaScript functions to simply ignore extra arguments.
The moddable SDK already has a driver for the HX711 but I found it to be very, very slow. E.g. over 300ms per reading, in part due to averaging that can't be disabled. This new driver is very simple and optimized for speed: it essentially just provides one
read()
function to read the current ADC value without performing any averaging. There is a second functionreadable()
to check that the chip can be read (it typically samples at 10Hz). This driver implements the read in C using modGPIO and performs a read in a couple of milliseconds.Conveniently the existing driver is under
modules/drivers
and this PR places code inmodules/drivers/sensors
so there is no clash.This driver is intended to conform to ECMA-419.