Skip to content
This repository was archived by the owner on Apr 9, 2026. It is now read-only.
Merged
Show file tree
Hide file tree
Changes from all commits
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
15 changes: 15 additions & 0 deletions MicrocontrollerCode/include/JoystickFunctions.h
Original file line number Diff line number Diff line change
Expand Up @@ -15,13 +15,28 @@ struct RefSpeed {
int8_t rightSpeed; ///< Speed of the right wheel.
};

/**
* Struct representing the reference displace
Comment thread
arturomatlin marked this conversation as resolved.
*/
struct RefDisplacement {
int8_t longDisp; ///< Forward/Backward displacement with + indicating forward
int8_t latDisp; ///< Side to side displacement with + indicating right
Comment on lines +21 to +23

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

Critical: int8_t will overflow with raw ADC displacement values.

Based on the implementation in MicrocontrollerCode/src/JoystickFunctions.cpp (lines 29-30), the displacement calculation produces values in the range of approximately -4400 to +4400 (from ADC readings 0-17390 minus offset 12900). Storing these in int8_t fields (range: -128 to 127) will cause severe integer overflow, resulting in incorrect displacement data being transmitted.

Apply this diff to use an appropriate integer type:

 struct RefDisplacement {
-    int8_t longDisp;      ///< Forward/Backward displacement with + indicating forward
-    int8_t latDisp;     ///< Side to side displacement with + indicating right
+    int16_t longDisp;      ///< Forward/Backward displacement with + indicating forward
+    int16_t latDisp;     ///< Side to side displacement with + indicating right
 };

Note: You'll also need to update the corresponding message field types in the ROS message definition to match.

};

/**
* Reads a value from the joystick connected to the ADC and returns the reference speeds
* @param adc An instance of the adc the joystick is connected to
* @return A RefSpeed for the wheelchair containing the wheel speeds and the direction
*/
RefSpeed joystickToSpeed(Adafruit_ADS1115 &adc);

/**
* Reads a value from the joystick connected to the ADC and returns the reference displacement
* @param adc An instance of the adc the joystick is connected to
* @return A RefDisplacement for the joystick
*/
RefDisplacement joystickToDisplacement(Adafruit_ADS1115 &adc);

template <typename T>
/**
* Custom clamp function to keep a value between to values
Expand Down
22 changes: 22 additions & 0 deletions MicrocontrollerCode/src/JoystickFunctions.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,28 @@
int diffParam = 30;
int deadzoneParam = 30;

RefDisplacement joystickToDisplacement(Adafruit_ADS1115 &adc){
int forwardJoystick = adc.readADC_SingleEnded(0); //a0 is forward/backward
int sidewaysJoystick = adc.readADC_SingleEnded(1); //a1 is left/right

RefDisplacement displacements;
/*
* Joystick middle values: ~8500
* a0 middle value: ~8500
* a1 middle value: ~8300
* a0 deadzone 10000 - 6500
* a1 deadzone 11000 - 6000
* Joystick Min: 0
* Joystick Max: 17390
* Output is a value -100 to 100 for the speed of the motor
*/

//Converting the speeds so they start around 0 and then go positive and negative
displacements.longDisp = forwardJoystick - (8500+4400); //The second value is used to zero it out when the ADC gain is set to 0 instead of the default (2/3)
displacements.latDisp = sidewaysJoystick - (8400+4400);
return displacements;
}
Comment on lines +12 to +32

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

Critical: Integer overflow and misleading comment.

This function has multiple critical issues:

  1. Integer overflow: The calculation forwardJoystick - (8500+4400) produces values in the range of approximately -4400 to +4400 (from ADC readings 0-17390 minus offset 12900), but these are assigned to int8_t fields (range: -128 to 127) in the RefDisplacement struct. This will cause severe overflow, corrupting the displacement data.

  2. Misleading comment: Line 25 states "Output is a value -100 to 100 for the speed of the motor" but the function neither scales to this range nor relates to motor speed—it's displacement data. The comment appears to be copy-pasted from joystickToSpeed without updating.

  3. No normalization: Unlike joystickToSpeed which normalizes and scales to ±100, this function returns raw centered ADC values without any processing.

Apply this diff to normalize displacement to a ±100 range and fix the comment:

 RefDisplacement joystickToDisplacement(Adafruit_ADS1115 &adc){
     int forwardJoystick = adc.readADC_SingleEnded(0); //a0 is forward/backward
     int sidewaysJoystick = adc.readADC_SingleEnded(1); //a1 is left/right
 
     RefDisplacement displacements;
     /*
      * Joystick middle values: ~8500
      * a0 middle value: ~8500
      * a1 middle value: ~8300
      * a0 deadzone 10000 - 6500
      * a1 deadzone 11000 - 6000
      * Joystick Min: 0
      * Joystick Max: 17390
-     * Output is a value -100 to 100 for the speed of the motor
+     * Output is a value -100 to 100 for joystick displacement
      */
 
-    //Converting the speeds so they start around 0 and then go positive and negative
-    displacements.longDisp = forwardJoystick - (8500+4400); //The second value is used to zero it out when the ADC gain is set to 0 instead of the default (2/3)
-    displacements.latDisp = sidewaysJoystick - (8400+4400);
+    // Center the joystick values around 0
+    forwardJoystick = forwardJoystick - (8500+4400);
+    sidewaysJoystick = sidewaysJoystick - (8400+4400);
+    
+    // Normalize to -100 to 100 range
+    const float MAX_INPUT = 13000.0f;
+    float longNorm = constrain(forwardJoystick / MAX_INPUT, -1.0f, 1.0f);
+    float latNorm = constrain(sidewaysJoystick / MAX_INPUT, -1.0f, 1.0f);
+    
+    displacements.longDisp = (int8_t)roundf(longNorm * 100.0f);
+    displacements.latDisp = (int8_t)roundf(latNorm * 100.0f);
     return displacements;
 }

Alternative: If you need the full raw ADC range, change the struct fields to int16_t as suggested in the header file review and keep the raw values. Choose based on whether downstream consumers need raw ADC values or normalized joystick position.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
RefDisplacement joystickToDisplacement(Adafruit_ADS1115 &adc){
int forwardJoystick = adc.readADC_SingleEnded(0); //a0 is forward/backward
int sidewaysJoystick = adc.readADC_SingleEnded(1); //a1 is left/right
RefDisplacement displacements;
/*
* Joystick middle values: ~8500
* a0 middle value: ~8500
* a1 middle value: ~8300
* a0 deadzone 10000 - 6500
* a1 deadzone 11000 - 6000
* Joystick Min: 0
* Joystick Max: 17390
* Output is a value -100 to 100 for the speed of the motor
*/
//Converting the speeds so they start around 0 and then go positive and negative
displacements.longDisp = forwardJoystick - (8500+4400); //The second value is used to zero it out when the ADC gain is set to 0 instead of the default (2/3)
displacements.latDisp = sidewaysJoystick - (8400+4400);
return displacements;
}
RefDisplacement joystickToDisplacement(Adafruit_ADS1115 &adc){
int forwardJoystick = adc.readADC_SingleEnded(0); //a0 is forward/backward
int sidewaysJoystick = adc.readADC_SingleEnded(1); //a1 is left/right
RefDisplacement displacements;
/*
* Joystick middle values: ~8500
* a0 middle value: ~8500
* a1 middle value: ~8300
* a0 deadzone 10000 - 6500
* a1 deadzone 11000 - 6000
* Joystick Min: 0
* Joystick Max: 17390
* Output is a value -100 to 100 for joystick displacement
*/
// Center the joystick values around 0
forwardJoystick = forwardJoystick - (8500 + 4400);
sidewaysJoystick = sidewaysJoystick - (8400 + 4400);
// Normalize to -100 to 100 range
const float MAX_INPUT = 13000.0f;
float longNorm = constrain(forwardJoystick / MAX_INPUT, -1.0f, 1.0f);
float latNorm = constrain(sidewaysJoystick / MAX_INPUT, -1.0f, 1.0f);
displacements.longDisp = (int8_t)roundf(longNorm * 100.0f);
displacements.latDisp = (int8_t)roundf(latNorm * 100.0f);
return displacements;
}


RefSpeed joystickToSpeed(Adafruit_ADS1115 &adc){
int forwardJoystick = adc.readADC_SingleEnded(0); //a0 is forward/backward
int sidewaysJoystick = adc.readADC_SingleEnded(1); //a1 is left/right
Expand Down
8 changes: 6 additions & 2 deletions MicrocontrollerCode/src/main.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -104,10 +104,14 @@ void loop() {
//TODO might want to figure out how to put these on core1 so that they can run in parallel
//uint32_t start = millis();
RefSpeed omegaRef{};
RefDisplacement thetaRef{};
if (!joystick_adc_error) {
omegaRef = joystickToSpeed(joystickAdc);
thetaRef = joystickToDisplacement(joystickAdc);
}




//uint32_t joystickTime = millis() - start;
USData usDistances{};
Expand Down Expand Up @@ -147,7 +151,7 @@ void loop() {

microRosTick();

transmitMsg(omegaRef, usDistances, pirSensors, fanSpeeds, imuData);
transmitMsg(thetaRef,omegaRef, usDistances, pirSensors, fanSpeeds, imuData);

if (currentMillis - lastErrorTime >= error_timer) {
lastErrorTime = currentMillis;
Expand All @@ -160,7 +164,7 @@ void loop() {
#elif ROS_DEBUG


transmitMsg(omegaRef);
transmitMsg(thetaRef,omegaRef);


#elif DEBUG
Expand Down
8 changes: 6 additions & 2 deletions MicrocontrollerCode/src/microRosFunctions.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -373,7 +373,9 @@ static void lidar_subscription_callback(const void *msgin){
}

#ifdef ROS
void transmitMsg(RefSpeed omegaRef, USData ultrasonicData, PIRSensors pirSensors, FanSpeeds fanSpeeds, IMUData imuData) {
void transmitMsg(RefDisplacement thetaRef, RefSpeed omegaRef, USData ultrasonicData, PIRSensors pirSensors, FanSpeeds fanSpeeds, IMUData imuData) {
sensorMsg.long_disp = thetaRef.longDisp;
sensorMsg.lat_disp = thetaRef.latDisp;
sensorMsg.left_speed = omegaRef.leftSpeed;
sensorMsg.right_speed = omegaRef.rightSpeed;
sensorMsg.ultrasonic_front_0 = ultrasonicData.us_front_0;
Expand Down Expand Up @@ -406,9 +408,11 @@ void transmitMsg(RefSpeed omegaRef, USData ultrasonicData, PIRSensors pirSensors

#elif ROS_DEBUG

void transmitMsg(RefSpeed omegaRef){
void transmitMsg(RefDisplacement thetaRef, RefSpeed omegaRef){
msg.left_speed = omegaRef.leftSpeed;
msg.right_speed = omegaRef.rightSpeed;
msg.long_disp = thetaRef.longDisp;
msg.lat_disp = thetaRef.latDisp;

RCSOFTCHECK(rclc_executor_spin_some(&executor, RCL_MS_TO_NS(10)));
}
Expand Down