-
Notifications
You must be signed in to change notification settings - Fork 4
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 support for G&G JJ100B scale #7
base: develop
Are you sure you want to change the base?
Add support for G&G JJ100B scale #7
Conversation
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.
Some initial feedback for now. I'll need to wrap up #5 first because it will make it easier for you to implement the work in this PR that allows for different UNIT_MAP
s for different scales.
trickler/main.py
Outdated
@@ -162,7 +162,7 @@ def main(config, args, pidtune_logger): | |||
parser.add_argument('--verbose', action='store_true') | |||
parser.add_argument('--auto_mode', action='store_true') | |||
parser.add_argument('--pid_tune', action='store_true') | |||
parser.add_argument('--target_weight', type=decimal.Decimal, default=0) | |||
parser.add_argument('--target_weight', type=decimal.Decimal, default=10) # default=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.
Please revert this change. You can keep it locally for testing if you like, but it should not go into the repo.
trickler/scales.py
Outdated
@@ -25,13 +25,13 @@ class Units(enum.Enum): | |||
|
|||
UNIT_MAP = { | |||
'GN': Units.GRAINS, | |||
'g': Units.GRAMS, | |||
'G': Units.GRAMS, |
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 change and the one below would break support for AND scales if committed. The problem is that I designed it around AND scales and need to refactor this sort of thing to allow other brands with different data formats and unit names to be separated.
I'll handle the separation in this other PR (#5 ), which I'll need to merge first so that it will make it more clear what you'll need to do for your scale.
trickler/scales.py
Outdated
# Set default values, which should be overwritten quickly. | ||
self.raw = b'' | ||
self.unit = Units.GRAINS | ||
self.resolution = decimal.Decimal(0.02) |
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 resolution correct for your scale?
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.
Negativ, resolution for grains = 0.01
|
||
# Note: The input buffer can fill up, causing latency. Clear it before reading. | ||
self._serial.reset_input_buffer() | ||
self._serial.write(b'\x1B\x70') |
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 you need to send a command before reading from the scale, please document what is being sent and why it's needed.
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.
In order to get a reading from the scale the hardware number (default 27dec = 0x1B) + 0x70 has to be send to the scale
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.
Got it. Have you tested this with your scale to see if it works? Is a delay needed after sending before the weight can be 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.
Yes, I have tested this with my scale. I put a delay for safety reasons but I will test if it works without delay too. Here is a sample code I used for reading and changing units on my scale:
import atexit
import serial
import time
ser = serial.Serial(port='/dev/ttyUSB0', baudrate=9600, timeout=0.1)
atexit.register(ser.close)
def change_units():
ser.write(b'\x1B\x73')
def read_units():
ser.write(b'\x1B\x70')
units = ser.readline().decode('utf-8')
units = units[9:12]
print(units)
return units
if name == "main":
while read_units() != " GN":
change_units()
time.sleep(1)
trickler/scales.py
Outdated
handler = handlers.get(status, noop) | ||
handler2 = handlers.get(fault, noop) | ||
handler(line) | ||
handler2(line) |
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.
There may be a better way to do this than calling both functions when only one is needed. I know that it's a little more complicated because of the data format. I don't have a suggestion off the top of my head.
At the very least, let's rename the functions to something more clear instead of handler
and handler2
and add some comments to document what is being done here and why.
trickler/scales.py
Outdated
resolution[Units.GRAINS] = decimal.Decimal(0.02) | ||
resolution[Units.GRAMS] = decimal.Decimal(0.001) |
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.
Are these resolutions correct for your scale?
trickler/scales.py
Outdated
@@ -187,6 +314,7 @@ def _serial_number(self, line): | |||
|
|||
SCALES = { | |||
'and-fx120': ANDFx120, | |||
'jj100b': GGjj100b, |
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.
Please match the naming convention and change the key to 'gg-jj100b'
trickler/scales.py
Outdated
@@ -196,9 +324,9 @@ def _serial_number(self, line): | |||
import helpers | |||
|
|||
parser = argparse.ArgumentParser(description='Test scale.') | |||
parser.add_argument('--scale', choices=SCALES.keys(), default='and-fx120') | |||
parser.add_argument('--scale', choices=SCALES.keys(), default='jj100b') |
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.
Please revert this.
trickler/scales.py
Outdated
parser.add_argument('--scale_port', default='/dev/ttyUSB0') | ||
parser.add_argument('--scale_baudrate', type=int, default='19200') | ||
parser.add_argument('--scale_baudrate', type=int, default='9600') |
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.
Please revert this.
Note: This PR will resolve ammolytics/projects#59 |
trickler/scales.py
Outdated
def update(self): | ||
"""Read from the serial port and update an instance of this class with the most recent values.""" | ||
handlers = { | ||
' GN': self._stable, |
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.
Should other units (such as grams, ounces, etc) be listed here as well?
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.
For the trickler project it is not necessary but my scale supports following units so the missing units could be implemented here:
grams (g), grains (GN), ounces (oz), carat (ct), troy ounce(ozt), pennyweight (dwt), pound (lb)
|
||
# Note: The input buffer can fill up, causing latency. Clear it before reading. | ||
self._serial.reset_input_buffer() | ||
self._serial.write(b'\x1B\x70') |
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.
Got it. Have you tested this with your scale to see if it works? Is a delay needed after sending before the weight can be read?
Please update your branch based on the latest changes in the |
Following up on this PR. Any chance you'll be able to update it so that it can be included into the next version? |
Hi Eric. Of course I can update it. I'm back from work at weekend then we
can give it a try (I'm sure I'll need your help)
Best regards
Patrick
Eric Higgins ***@***.***> schrieb am Mo., 13. Nov. 2023,
18:35:
… Following up on this PR. Any chance you'll be able to update it so that it
can be included into the next version?
—
Reply to this email directly, view it on GitHub
<#7 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AJAFSHI2GS7MAVXGCAWV4XDYEJK43AVCNFSM6AAAAAAVTNA4ISVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQMBYGY2TAOJQGA>
.
You are receiving this because you authored the thread.Message ID:
***@***.***>
|
No description provided.