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

NAU7802 implementation and supporting refactor #33

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions MANIFEST.in
Original file line number Diff line number Diff line change
Expand Up @@ -2,3 +2,4 @@ include README.md
recursive-include filament_scale_enhanced/templates *
recursive-include filament_scale_enhanced/translations *
recursive-include filament_scale_enhanced/static *
recursive-include filament_scale_enhanced/scale *
179 changes: 154 additions & 25 deletions filament_scale_enhanced/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,24 +2,18 @@
from __future__ import absolute_import

import octoprint.plugin
import flask
import time
from octoprint.access.permissions import Permissions, ADMIN_GROUP

from .fse_version import VERSION as __version__ # noqa: F401
from .hx711 import HX711

try:
import RPi.GPIO as GPIO
except (ModuleNotFoundError, RuntimeError):
import Mock.GPIO as GPIO # noqa: F401


# pylint: disable=too-many-ancestors
class FilamentScalePlugin(octoprint.plugin.SettingsPlugin,
octoprint.plugin.AssetPlugin,
octoprint.plugin.TemplatePlugin,
octoprint.plugin.StartupPlugin):

hx = None
t = None
octoprint.plugin.StartupPlugin,
octoprint.plugin.SimpleApiPlugin):

@staticmethod
def get_template_configs():
Expand All @@ -30,12 +24,16 @@ def get_template_configs():
@staticmethod
def get_settings_defaults():
return dict(
tare=8430152,
reference_unit=-411,
# chips=["hx711", "nau7802", "none"],
chip="nau7802",
Copy link
Owner

Choose a reason for hiding this comment

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

Might be worth keeping the default as the cheaper chip, that tends to be what most implementations seem to use in all the guides. A default with the other type may confuse beginners.

Suggested change
chip="nau7802",
chip='hx711',

Copy link
Author

@fivesixzero fivesixzero Jun 19, 2022

Choose a reason for hiding this comment

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

Definitely agree there. That default was a debug change that I forgot to change out before commit. 🤦

  • Assure that default chip is hx711, not nau7802

offset=0.0,
cal_factor=1.0,
hx_clockpin=21,
hx_datapin=20,
nau_bus_id=1,
lastknownweight=0,
spool_weight=200,
clockpin=21,
datapin=20,
lastknownweight=0
update_delay=3.0
)

@staticmethod
Expand All @@ -46,19 +44,63 @@ def get_assets():
less=["less/filament_scale.less"]
)


def on_startup(self, host, port): # pylint: disable=unused-argument
self.hx = HX711(20, 21)
self.hx.set_reading_format("LSB", "MSB")
self.hx.reset()
self.hx.power_up()
self.t = octoprint.util.RepeatedTimer(3.0, self.check_weight)
self.t.start()
self.time = time.time()

chip = self._settings.get(["chip"])
offset = float(self._settings.get(["offset"]))
cal_factor = float(self._settings.get(["cal_factor"]))

self._logger.info("Setting up scale, chip: [{}], offset: [{}], cal_factor: [{}]".format(chip, offset, cal_factor))

if chip == "nau7802":
from .scale.nau7802 import NAU7802

bus_id = int(self._settings.get(["nau_bus_id"]))

self._logger.debug("Initializing NAU7802, bus_id: [{}]".format(bus_id))
self.scale = NAU7802(offset, cal_factor, bus_id)
self._logger.debug("Initialized NAU7802, is_ready: [{}]".format(self.scale.is_ready()))
elif chip == "hx711":
from .scale.hx711 import HX711

clock_pin = self._settings.get(["hx_clockpin"])
data_pin = self._settings.get(["hx_datapin"])

self._logger.debug("Initializing HX711, clock_pin: [{}], data_pin: [{}]".format(clock_pin, data_pin))
self.scale = HX711(offset, cal_factor, clock_pin, data_pin)
self._logger.debug("Initialized HX711, is_ready: [{}]".format(self.scale.is_ready()))
else:
self.scale = None
Copy link
Owner

Choose a reason for hiding this comment

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

It'll save some boiler plate and give room for different backends in the future (maybe say an external arduino), if we from . import scale at the top of the file, then use getattr and pass the whole config payload into the library. Let each implementation handle parsing it during instantiation. Along with the logging statements. Potentially a try/except for if the backend configured doesn't exist

Suggested change
if chip == "nau7802":
from .scale.nau7802 import NAU7802
bus_id = int(self._settings.get(["nau_bus_id"]))
self._logger.debug("Initializing NAU7802, bus_id: [{}]".format(bus_id))
self.scale = NAU7802(offset, cal_factor, bus_id)
self._logger.debug("Initialized NAU7802, is_ready: [{}]".format(self.scale.is_ready()))
elif chip == "hx711":
from .scale.hx711 import HX711
clock_pin = self._settings.get(["hx_clockpin"])
data_pin = self._settings.get(["hx_datapin"])
self._logger.debug("Initializing HX711, clock_pin: [{}], data_pin: [{}]".format(clock_pin, data_pin))
self.scale = HX711(offset, cal_factor, clock_pin, data_pin)
self._logger.debug("Initialized HX711, is_ready: [{}]".format(self.scale.is_ready()))
else:
self.scale = None
self.scale = getattr(scale, chip)(settings=self._settings)

Copy link
Author

@fivesixzero fivesixzero Jun 19, 2022

Choose a reason for hiding this comment

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

That's a great idea and I'm all for simplifying this code.

  • Move per-device instantiation details to scale library
  • Implement config parsing in scale library to determine which subclass to init and handle per-subclass variables
  • Use getarr() to init self.scale with the active config using the above implementation during on_startup()


if self.scale.is_ready():
timer_delay = float(self._settings.get(["update_delay"]))
self._logger.info("Setting up RepeatedTimer for check_weight(), delay: [{}]".format(timer_delay))
self.t = octoprint.util.RepeatedTimer(timer_delay, self.check_weight)
self.t.start()
else:
self.t = None
self._logger.warn("Skipping RepatedTimer setup for check_weight(), scale is not ready")


def check_weight(self):
self.hx.power_up()
v = self.hx.read()
chip = self._settings.get(["chip"])
if chip == "hx711":
self.scale.enable()

v = self.scale.get_weight()

self._logger.debug("weight: [{}]".format(v))

self._settings.set(["lastknownweight"], v)
self._settings.save()

self._plugin_manager.send_plugin_message(self._identifier, v)
self.hx.power_down()

if chip == "hx711":
self.scale.disable()
Copy link
Owner

Choose a reason for hiding this comment

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

I can't recall why it's necessary to power up/down here for the hx11. But maybe implementing a initial_check_weight method, that by default calls get_weight and is overriden by the hx711 chip might be a touch cleaner

Something like this in the hx11 class

def initial_check_weight(self) -> float:
    self.enable()
    weight = self.get_weight()
    self.disable()
    return weight

Copy link
Author

@fivesixzero fivesixzero Jun 19, 2022

Choose a reason for hiding this comment

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

This was one of the first parts of the code I dug into and this is an example of just trying to keep things as identical as possible to the original implementation before making more changes.

When I was working on the CircuitPython HX711 GPIO bitbang driver I definitely observed some oddities around 'enable' vs 'disable' state management. In that driver's case I basically just abandoned all hope of tracking a 'powered up' vs 'powered down' state and pared the implementation down to the barest of basics, the raw ADC count read. That has its own downside - leaving the clock line low means the first read-in could provide a stale reading - but its easy enough to just read a few dozen counts, throw away the first read (or just remove outliers) before averaging.

Even with that knowledge though there's always going to be a risk of severe timing issues causing odd behavior in general, which is kinda unavoidable when bitbanging from high in the stack like this. Even with the much more lightweight CircuitPython, platforms like nrf and esp32 (which do BLE/WiFi management in background threads) can see some weird timing issues pop up that can totally wreck the driver's reliability. So this kind of mitigation doesn't surprise me or seem out of place, even if I'm not sure precisely why it was included.

Anyway, long story short, I think this "state toggle" should probably stick around (for now, at least). And I agree that it probably belongs in the scale.hx711 wrapper rather than here in the main plugin code. :)

  • Move hx711 state toggle on read out of the check_weight() and into the scale driver wrapper



# pylint: disable=line-too-long
def get_update_information(self):
Expand All @@ -83,6 +125,93 @@ def get_update_information(self):
)


def scale_tare(self) -> float:
current_offset = self._settings.get(["offset"])
self._logger.info("Tare started, current offset: [{}]".format(current_offset))

new_offset = self.scale.tare()

self._settings.set(["offset"], new_offset)
self._settings.save()
self._logger.info("Tare complete, new_offset: [{}]".format(new_offset))

return new_offset


def scale_calibrate(self, weight: float) -> float:
current_cal_factor = self._settings.get(["cal_factor"])
self._logger.info("Calibration start, weight: [{}], current cal_factor: [{}]".format(weight, current_cal_factor))

new_cal_factor = self.scale.calibrate(weight)

self._settings.set(["cal_factor"], new_cal_factor)
self._settings.save()
self._logger.info("Calibration complete, weight: [{}], new_cal_factor: [{}]".format(weight, new_cal_factor))

return new_cal_factor


def scale_weight(self) -> float:
self._logger.debug("Scale last_value: [{}], last_value_time: [{}]".format(self.scale.last_value, self.scale.last_value_time))
if time.time - self.scale.last_value_time < 5:
weight = self.scale.last_value
else:
weight = self.scale.get_weight()

self._logger.info("Scale weight: [{}]".format(weight))

return weight


def on_api_get(self, request) -> str:
if time.time() - self.time < 0.1:
return flask.make_response("Slow down!", 200)

command = request.args.get('command')
value = request.args.get('value')

if command == "tare":
self.time = time.time()

new_offset = self.scale_tare()
self._logger.info("API tare, new_offset: [{}]".format(new_offset))

return flask.make_response(str(new_offset), 200)
elif command == "calibrate":
self.time = time.time()

try:
value = float(value)
except ValueError:
self._logger.info("API calibrate value error, cannot convert value to float")
return flask.make_response("Invalid calbiration weight value, cannot convert to float", 400)

if value < 0:
self._logger.info("API calibrate value error, cannot calibrate against zero or negative")
return flask.make_response("Invalid calibration weight value, must be greater than zero", 400)

new_cal_factor = self.scale_calibrate(value)
self._logger.info("API calibrate, new_cal_factor: [{}]".format(new_cal_factor))

return flask.make_response(str(new_cal_factor), 200)
elif command == "weight":
self.time = time.time()

weight = self.scale_weight()
self._logger.info("API weight: [{}]".format(weight))

return flask.make_response(str(weight), 200)
elif command == "status":
self.time = time.time()

status = self.scale.status()
self._logger.info("API status: [{}]".format(status))

return flask.make_response(str(status), 200)

return flask.make_response("ok", 200)


__plugin_name__ = "Filament Scale" # pylint: disable=global-variable-undefined
__plugin_pythoncompat__ = ">=3,<4" # pylint: disable=global-variable-undefined

Expand Down
Loading