Here's my code. Wanted: constructive criticism & best practice recommendations.

Hello everyone.

I’ll start from the end: I got it working! However, there are a few moments that I’d like to check with the experts.

I need any advice on code optimization and some help regarding the delayed response. Whenever I apply a load to the sensor, it takes a quarter of a second before I see the result.

My setup:
Load Cell > HX711 Amplifier > Arduino Pro Micro > PC (via USB)

Arduino is detected by Windows as a Gamepad with one Axis and one button.

Picture

There is a micro-switch connected to Pin7, RST and GND on Arduino. The switch has 3 positions: two for Arduino operation modes and one to reset Arduino. The Reset is in the middle position, so that every time I switch between the two operation modes, the device is reset first.

Picture

First operation mode is BUTTON mode: the gamepad button is activated when the load applied to the load cell reaches a certain level. The button remains depressed until the load is falling below that threshold.

Picture

Second operation mode is AXIS mode: the load data is mapped to the gamepad axis.

Picture

Here’s the code:

#include <Q2HX711.h>

const byte hx711_data_pin = SDA;
const byte hx711_clock_pin = SCL;

Q2HX711 hx711(hx711_data_pin, hx711_clock_pin);

#include <Joystick.h>

// Report ID, Joystick Type, Button Count, Hat Switch Count, X-Axis, Y-Axis, Z-Axis, Rx, Ry, Rz, rudder, throttle, accelerator, brake, steering.
// Defining a gamepad with 1 button and a Z-Axis present.
Joystick_ Joystick(JOYSTICK_DEFAULT_REPORT_ID,JOYSTICK_TYPE_GAMEPAD, 1, 0, false, false, true, false, false, false, false, false, false, false, false);

bool modeAxis;

void setup()
{
  pinMode(7, INPUT_PULLUP); // The initial state of pin7 determines the function that will be executed inside the loop()
  modeAxis = (digitalRead(7) == LOW); // If pin7 is grounded then modeAxis = true, if not then modeAxis = false (controlled via SP3T on-on-on switch).

  Serial.begin(9600); //Initialize Serial
  Joystick.begin(); // Initialize Joystick Library
  Joystick.setZAxisRange(0, 1023); // Z-Axis range 1024 values (10-bit)
}

void loop()
{
  //Serial.println(hx711.read());
  if(modeAxis) // If modeAxis = false then execute AXIS mode
    {
      long mappedLoad = (hx711.read()-8584200)/400; // In resting state the readings report around 8584200 conventional units. Substracted to zero out the units and divided by 400 to increase the step and meet the axis range (0-1023).
      Serial.println(mappedLoad);
      Joystick.setZAxis(mappedLoad);// Work in progress for AXIS mode.
    }
  else // If modeAxis = false then execute BUTTON mode
    {
      if (hx711.read()>8700000) {Joystick.setButton(0, 1);} else {Joystick.setButton(0, 0);} // Button is pressed as soon as the readings from load cell reach 8700000+
    }
}
  1. Is there anything obvious that I should change in order to get faster response times?
  2. The HX711 Amp has a 24-bit ADC, while Arduino is limited to 10-bit. Which board ends up converting the signal? Can this 24/10 mismatch cause any slowness if not taken care of properly in the code (some sort of downscaling?)
  3. If Arduino Pro Micro is capable of outputting 10-bit of data (1024 values?) then how come I can see 7-digit load values coming out of the serial monitor?

Thanks!

  1. Remove the Serial.println() in the middle. Although that won't really be making a significant difference.

  2. You aren't using analogRead() so what does 10 bits have to do with anything?

  3. Because the HX711 is doing the A-D conversion. The Arduino gets digital data from the chip.

I don't know about that library. It looks like a homegrown I2C protocol but I haven't read the HX711 datasheet to understand what it's really doing.

I'd look at the following line and put some profiling around it - how long does it usually have to wait for the thing to be ready to send?

   while (!readyToSend());

Perhaps you can set the HX711 to a faster conversion mode. You probably don't need 24 bits of accuracy. If it's only asked to convert 10 bits then it may be a thousand times faster.

#include <Q2HX711.h>
////// I'd put all the library includes first, rather than down in with the declarations.
#include <Joystick.h>

//////// No need to re-name the named pins.  I would remove these two lines and use SDA and SCL in the declaration.
const byte hx711_data_pin = SDA;
const byte hx711_clock_pin = SCL;

/////// It is good to distinguish global variables by having them start with a capital letter.  I'd change it to "Hx711"
Q2HX711 hx711(hx711_data_pin, hx711_clock_pin);

////// You did not name Pin 7:
const byte ModeAxisPin = 7;

// Report ID, Joystick Type, Button Count, Hat Switch Count, X-Axis, Y-Axis, Z-Axis, Rx, Ry, Rz, rudder, throttle, accelerator, brake, steering.
// Defining a gamepad with 1 button and a Z-Axis present.
Joystick_ Joystick(JOYSTICK_DEFAULT_REPORT_ID, JOYSTICK_TYPE_GAMEPAD, 1, 0, false, false, true, false, false, false, false, false, false, false, false);

/////// It is good to distinguish global variables by having them start with a capital letter.  I'd change it to "ModeAxis"
bool modeAxis;

void setup()
{
  ////// Use the name ModeAxisPin in place of '7'
  pinMode(7, INPUT_PULLUP); // The initial state of pin7 determines the function that will be executed inside the loop()
  ////// Use the name ModeAxisPin in place of '7'
  modeAxis = (digitalRead(7) == LOW); // If pin7 is grounded then modeAxis = true, if not then modeAxis = false (controlled via SP3T on-on-on switch).

  Serial.begin(9600); //Initialize Serial
  Joystick.begin(); // Initialize Joystick Library
  Joystick.setZAxisRange(0, 1023); // Z-Axis range 1024 values (10-bit)
}

void loop()
{
  //Serial.println(hx711.read());
  if (modeAxis) // If modeAxis = false then execute AXIS mode
  {
//////  Give the magic constant a name!
//////  It won't fit in an int (32,767 max) so you should mark the constant as 'long' (8584200L) or unsigned long (8584200UL)
    long mappedLoad = (hx711.read() - 8584200) / 400; // In resting state the readings report around 8584200 conventional units. Substracted to zero out the units and divided by 400 to increase the step and meet the axis range (0-1023).
////// If the range is 0 to 1023 you can store it in an 'int'.  What type does .setZAxis() take?
    Serial.println(mappedLoad);
    Joystick.setZAxis(mappedLoad);// Work in progress for AXIS mode.
  }
  else // If modeAxis = false then execute BUTTON mode
  {
//////  What is the magical value '8700000'???  Give it a name!
//////  It won't fit in an int (32,767 max) so you should mark the constant as 'long' (8700000L) or unsigned long (8700000UL)
    if (hx711.read() > 8700000) {
      Joystick.setButton(0, 1); // Button is pressed as soon as the readings from load cell reach 8700000+
    } else {
      Joystick.setButton(0, 0);
    }
  }
}

General recommendation:
Use Auto-Format often.
Assign names to pins in pin number order. Makes it easy to spot when a pin is used twice and what pins are available.
Set Preferences to Compiler warnings ALL. Fix as many warnings as you can. Sometimes they show coding mistakes that can cause your sketch to fail.

You have magic constants like 8700000 and 8584200 and 400 dotted around the code - they are
meaningless to me - #define them with well chosen names.

BTW you mean 8700000L and 8584200L since 8700000 cannot be held in an int, and
perhaps since these are 24 bit binary values a hex constant is more appropriate.

Using 9600 baud is going to slow everything down a lot if you add serial debugging - 115200
baud is available and supported by the serial window, so use it.

Forget the Arduino ADC, you are not using it. You are pulling 24 bit values from the HX711
directly as digital values.

  1. If Arduino Pro Micro is capable of outputting 10-bit of data (1024 values?) then how come I can see 7-digit load values coming out of the serial monitor?

a) The Arduino outputs 8 bit PWM values, it can read 10bit analog values but that's nothing to do
with output. Your 7 digit values are the 24 bit values direct from the HX711.

MarkT:
#define them with well chosen names.

Or the safer C++ alternative, const :slight_smile:

MarkT:
BTW you mean 8700000L and 8584200L since 8700000 cannot be held in an int

Doesn't matter, compiler will already use a long because the value doesn't fit an int.

I miss a link to the HX711 library you use.

To speed things up, you could change the library to give you access to the last weight and the averange
in a nonblocking manner.

Hello everyone and thanks for the feedback. I am a beginner and this is my very first sketch and there's been a lot of digging lately trying to learn and understand libraries (.h and .cpp) and their structure. There are a lot of suggestions that are very new to me and I will need to do even more research to understand what you mean, but I'll get there.

Meanwhile, can you please help me understand the bit-ness of HX711 and Arduino? Similar projects are using transducers (PSI) to measure the hydraulic pressure created by the load. Arduino reads the analog values of pressure sensor and convert it to digital output, ending up with 0-1023 value resolution.

In my case the HX711 is performing the ADC conversion (24-bit) and sends the data to Arduino over the serial interface. Can you please shed more light on what happens on Arduino side? Is it able to re-transmit the 24-bit data to the PC over USB?
Windows shouldn't need more than 4096 values (12-bit) for the Axis. Is it possible to throttle the bit-ness down from 24-bit to 12-bit? Will it have any benefits responsiveness wise? And if possible, should that be performed on HX711 or Arduino?

My terminology is probably very non-technical, therefore I apologize from start.

If you want to know more about the capabilities of a device have a look at the datasheet.

If you want to see what happens on the Arduino side, have a look at the library you picked.

The chip is quite primitive and does all the work,
basically signaling "busy" and shifting out the 24 bit result on request if ready.
It does that 8 10 times a second but can be configured to 80 times (via hardware).
If the modification is simple depends on the breakout board. (I use 10 SPS only)

The HX711 can be run in a non-blocking fashion, but all libraries I found block.
Waiting for a measurement with the standard 10 SPS slows down your program,
especially if you want to averange some.

MorganS:

  1. Remove the Serial.println() in the middle. Although that won’t really be making a significant difference.

  2. You aren’t using analogRead() so what does 10 bits have to do with anything?

  3. Because the HX711 is doing the A-D conversion. The Arduino gets digital data from the chip.

I don’t know about that library. It looks like a homegrown I2C protocol but I haven’t read the HX711 datasheet to understand what it’s really doing.

I’d look at the following line and put some profiling around it - how long does it usually have to wait for the thing to be ready to send?

   while (!readyToSend());

Perhaps you can set the HX711 to a faster conversion mode. You probably don’t need 24 bits of accuracy. If it’s only asked to convert 10 bits then it may be a thousand times faster.

  1. I will definitely remove this after I am done working on Axis part. I will not need any Serial.Print in the code at all. Does this mean I can also remove Serial.begin(9600)? Or is it required since the communication from HX711 to Arduino is over serial?

2&3 Thanks for pointing this out. I wasn’t sure where the conversion happened.

=======================
Q2HX711.h:

#ifndef Q2HX711_h
#define Q2HX711_h
#include "Arduino.h"

class Q2HX711
{
  private:
    byte CLOCK_PIN;
    byte OUT_PIN;
    byte GAIN;
    void setGain(byte gain = 128);
  public:
    Q2HX711(byte output_pin, byte clock_pin);
    virtual ~Q2HX711();
    bool readyToSend();
    long read();
};

#endif /* Q2HX711_h */

==========================
Q2HX711.cpp:

#include <Arduino.h>
#include "Q2HX711.h"

Q2HX711::Q2HX711(byte output_pin, byte clock_pin) {
  CLOCK_PIN  = clock_pin;
  OUT_PIN  = output_pin;
  GAIN = 1;
  pinMode(CLOCK_PIN, OUTPUT);
  pinMode(OUT_PIN, INPUT);
}

Q2HX711::~Q2HX711() {
}

bool Q2HX711::readyToSend() {
  return digitalRead(OUT_PIN) == LOW;
}

void Q2HX711::setGain(byte gain) {
  switch (gain) {
    case 128:
      GAIN = 1;
      break;
    case 64:
      GAIN = 3;
      break;
    case 32:
      GAIN = 2;
      break;
  }

  digitalWrite(CLOCK_PIN, LOW);
  read();
}

long Q2HX711::read() {
   while (!readyToSend());

  byte data[3];

  for (byte j = 3; j--;) {
      data[j] = shiftIn(OUT_PIN,CLOCK_PIN, MSBFIRST);
  }

  // set gain
  for (int i = 0; i < GAIN; i++) {
    digitalWrite(CLOCK_PIN, HIGH);
    digitalWrite(CLOCK_PIN, LOW);
  }

  data[2] ^= 0x80;
  return ((uint32_t) data[2] << 16) | ((uint32_t) data[1] << 8) | (uint32_t) data[0];
}

Your last point about conversion rate on HX711 is very interesting. Do you think this would be possible and would help? As long as the bottleneck is not Arduino, I assume?

I had a quick scan of the datasheet. The one I found says it does either 10 samples per second or 80. It definitely should not be making you wait a quarter of a second. It's probably a good idea to shift it up to 80 samples.

Of course it's possible to send all 24 bits to the PC. Just make sure you always use intermediate variables bigger than this. A long integer is usually 32 bits. To be absolutely clear you want a 32-bit variable, use uint32_t or int32_t (The "u" stands for "unsigned".)

To convert a 24-bit variable into 10 bits, throwing away the extra bits and making it small enough to fit into a 16-bit integer, just bit-shift the unwanted bits away:

 uint16_t dividedValue = hx711.read() >> 14; //this is equivalent to dividing by 16,384

johnwasser:

#include <Q2HX711.h>
////// I’d put all the library includes first, rather than down in with the declarations.
DONE!
//////// No need to re-name the named pins. I would remove these two lines and use SDA and SCL in the declaration.
DONE!
/////// It is good to distinguish global variables by having them start with a capital letter. I’d change it to “Hx711”
DONE!
////// You did not name Pin 7:
DONE!
/////// It is good to distinguish global variables by having them start with a capital letter. I’d change it to “ModeAxis”
DONE!
////// Use the name ModeAxisPin in place of ‘7’
DONE!
////// Give the magic constant a name!
Work in progress, I might not need to use the magic number at all to zero the counter.
////// It won’t fit in an int (32,767 max) so you should mark the constant as ‘long’ (8584200L) or unsigned long (8584200UL)
////// If the range is 0 to 1023 you can store it in an ‘int’.
I might change the range to Joystick.setZAxisRange(0, 16383);
//////What type does .setZAxis() take?
void setZAxis(int16_t value);

I want to find out how much step resolution is required by Gamepads, I think 16bit for Axis is an overkill for Windows. Then I will adjust all the variables to the desired type.

At this point I need to find out and learn how to properly trim the 24-bit coming from HX711 down to 12 - 16 bit data stream.

Thank you very much!!!

MorganS:
I had a quick scan of the datasheet. The one I found says it does either 10 samples per second or 80. It definitely should not be making you wait a quarter of a second. It's probably a good idea to shift it up to 80 samples.

Of course it's possible to send all 24 bits to the PC. Just make sure you always use intermediate variables bigger than this. A long integer is usually 32 bits. To be absolutely clear you want a 32-bit variable, use uint32_t or int32_t (The "u" stands for "unsigned".)

To convert a 24-bit variable into 10 bits, throwing away the extra bits and making it small enough to fit into a 16-bit integer, just bit-shift the unwanted bits away:

 uint16_t dividedValue = hx711.read() >> 14; //this is equivalent to dividing by 16,384

Thanks for the help!
I think 12 bit (4096 values) for an Axis is more than enough. So I assume the conversion should look like this:

 uint16_t dividedValue = hx711.read() >> 12; // since 24-12=12

Correct?

Most likely I won't need to perform the operation with the "magic number 8584200". Do I just throw your conversion code right before the Joystick.setZAxis so the conversion happens with every operation, or can I optimize the code and take the conversion outside of the loop()? Or it won't make a difference?

Thanks!

MarkT:
Forget the Arduino ADC, you are not using it. You are pulling 24 bit values from the HX711
directly as digital values.
a) The Arduino outputs 8 bit PWM values, it can read 10bit analog values but that's nothing to do
with output. Your 7 digit values are the 24 bit values direct from the HX711.

So over the USB Arduino can send all 24-bits correct?

Using 9600 baud is going to slow everything down a lot if you add serial debugging - 115200
baud is available and supported by the serial window, so use it.

NOTED!

Whandall:
If you want to know more about the capabilities of a device have a look at the datasheet.

If you want to see what happens on the Arduino side, have a look at the library you picked.

The chip is quite primitive and does all the work,
basically signaling "busy" and shifting out the 24 bit result on request if ready.
It does that 8 10 times a second but can be configured to 80 times (via hardware).
If the modification is simple depends on the breakout board. (I use 10 SPS only)

The HX711 can be run in a non-blocking fashion, but all libraries I found block.
Waiting for a measurement with the standard 10 SPS slows down your program,
especially if you want to averange some.

Somehow I missed your post. Thanks for the info! 10 samples per second is not much at all, that's unfortunate.

Just read all values when they are available in a seperate routine (called from loop),
cache the value in the object and access it whenever needed.

That way you don't have to wait for anything.

I put the read values in a history buffer to get averanges, so its a little more complex

bool NBHX711::update() {
  bool retVal = false;
    if (isReady()) {
      retVal = true;
      curr = nextIndex(curr);
      putData(histBuffer + curr);
    }
    return retVal;
}

maxturcan:
10 samples per second is not much at all, that's unfortunate.

The HX711 is designed to build scales that are not so fast, and have trouble being much faster
because of inertia and mechanical oscillations.

You can modify your module to get 80 samples, maybe that's enough for your joystick application.

Whandall:
The HX711 is designed to build scales that are not so fast, and have trouble being much faster
because of inertia and mechanical oscillations.

You can modify your module to get 80 samples, maybe that’s enough for your joystick application.

Sorry, I was extra busy last week. I got the 80SPS working from the HX711, how the serial print is shooting values crazy fast and the axis response in Windows is WAAY faster. Thanks again for the direction.

I have another interesting problem that I am trying to resolve. Again, I might be using primitive language and I promise, I looked through the Joystick and HX711 libraries, I understand most of it, but some parts remain a black box for me, especially the Joystick library that is using HID.h.

The problem:

Whenever I use Windows built-in axis calibration tool, the “axis travel” bar is going crazy. Picture below just for reference.

The values below the bar are steady, not jumping places at all, but the bar itself is flashing very fast, as if it was outputting a real value, then the max value, real value, max value etc. So the bar refreshes very fast but one refresh cycle it’s full, and the nex one it’s in the middle, or zero (depending how I change the zAxis range in the code).

So I am thinking it could somehow be related that the read value is reset to a default zAxis value with each new cycle of the loop().

Do you think this might be possible?

Here’s the code (note, work in progress, some of the suggestions haven’t been applied yet):

#include <Q2HX711.h>
#include <Joystick.h>

Q2HX711 Hx711(SDA, SCL);

const byte ModeAxisPin = 7;

// Report ID, Joystick Type, Button Count, Hat Switch Count, X-Axis, Y-Axis, Z-Axis, Rx, Ry, Rz, rudder, throttle, accelerator, brake, steering.
// Defining a gamepad with 1 button and a Z-Axis present.
Joystick_ Joystick(JOYSTICK_DEFAULT_REPORT_ID,JOYSTICK_TYPE_GAMEPAD, 1, 0, false, false, true, false, false, false, false, false, false, false, false);

bool ModeAxis;
int16_t dividedValue;
const int16_t ButtonOn = -31200;

void setup()
{
  pinMode(ModeAxisPin, INPUT_PULLUP); // The initial state of pin7 determines the function that will be executed inside the loop()
  ModeAxis = (digitalRead(ModeAxisPin) == LOW); // If pin7 is grounded then modeAxis = true, if not then modeAxis = false (controlled via SP3T on-on-on switch).

  Serial.begin(115200); //Initialize Serial
  Joystick.begin(); // Initialize Joystick Library
  Joystick.setZAxisRange(-32767, 32767); // Z-Axis range 16384 values (16-bit)
}

void loop()
{
  Serial.println(dividedValue); // troubleshooting purposes, will remove later
  if(ModeAxis) // If modeAxis = false then execute AXIS mode
    {
      dividedValue = Hx711.read() >> 8; //remove 8 bits (16 remaining)
      Joystick.setZAxis(dividedValue);// Work in progress for AXIS mode.
    }
  else // If modeAxis = false then execute BUTTON mode
    {
      dividedValue = Hx711.read() >> 8; //remove 8 bits (16 remaining)
      if (dividedValue > ButtonOn) {Joystick.setButton(0, 1);} else {Joystick.setButton(0, 0);} // Button goes ON as soon as the readings from load cell reach the ButtonOn threshold
    }
}

Also, side question. I have this line of code in 2 places, for each modeAxis case:

dividedValue = Hx711.read() >> 8; //remove 8 bits (16 remaining)

What would be a better practice - move it in front of the IF so that the reading is taken regardless if the IF case, or leave it inside each IF case? In my opinion leaving this line closer to THEN, would provide more real-time readings than having the reading placed higher in the code. Or does it make any difference?

Thanks!

Follow the execution path. If ModeAxis is true then it has to remove 8 bits. If it's false then it has to do that anyway, so it doesn't matter either way.

You could optimise it a little by making your ButtonOn threshold a 32-bit value (bit-shift by 8 where you assign it a value) and then you don't need the bitshift in the main loop. But that's going to save you only 2 or 3 clock cycles. Like, less than a millionth of a second. That's not a significant speedup when you are looking at only 80 samples per second.

When you look at the raw output from your serial print, do the numbers jump around?

The axis effect could be a drawing problem of a progress bar windows control that tries to display 200%.

Somewhere there is a too small max value I guess.

Whandall:
The axis effect could be a drawing problem of a progress bar windows control that tries to display 200%.

Somewhere there is a too small max value I guess.

MorganS:
Follow the execution path. If ModeAxis is true then it has to remove 8 bits. If it's false then it has to do that anyway, so it doesn't matter either way.

You could optimise it a little by making your ButtonOn threshold a 32-bit value (bit-shift by 8 where you assign it a value) and then you don't need the bitshift in the main loop. But that's going to save you only 2 or 3 clock cycles. Like, less than a millionth of a second. That's not a significant speedup when you are looking at only 80 samples per second.

When you look at the raw output from your serial print, do the numbers jump around?

Gentlemen, I just uploaded a small video with the problem.

Apologies for the background music (in case you don't like it), I have no idea how to disable the sound in this recording app.

200% etc is normal for this calibration tool. It basically takes your initial state and marks it as zero, and then you pull your joystick/handle to the max and the tool marks your max as 100%. The percentage displayed during the calibration is misleading even for true gamepads. It's always 100%+.

It is worth mentioning that the calibration does it's job and I end up with the proper travel.

But this bug just tells me that something might be wrong in the code. Please have a look at the video and let me know what you think.