Error compiling a sketch to class/library conversion

Question

I'm trying to take a sketch and turn it into a class for a library I'm making, but I'm getting compile issues.

This is not a duplicate as suggested. as i explain in the comment and my answer. Thank you all for your help - as I am new to creating libraries - figured I would attempt something correctly for a change :)

Here is the original sketch: Gotten here

#include <ros.h>
#include <sensor_msgs/BatteryState.h>
#define K 0.00472199
#define CELLS 6
#define MAX_CELLS 12
double cell_const[MAX_CELLS] = 
{
 1.0000,
 2.1915,
 2.6970,
 4.1111,
 4.7333,
 6.6000,
 6.6000,
 7.8293,
 8.4667,
 9.2353,
 11.0000,
 11.0000
};
ros::NodeHandle nh;
sensor_msgs::BatteryState batt_state;
ros::Publisher batteryState("battery_state", &batt_state);
void setup() 
{
 // Initialize the ROS node.
 nh.initNode();
 nh.advertise(batteryState);
 // Populate battery parameters.
 batt_state.design_capacity = 2200; // mAh
 batt_state.power_supply_status = 2; // discharging
 batt_state.power_supply_health = 0; // unknown
 batt_state.power_supply_technology = 3; // LiPo
 batt_state.present = 1; // battery present
 batt_state.location = "Crawler"; // unit location
 batt_state.serial_number = "ABC_0001"; // unit serial number
 batt_state.cell_voltage = new float[CELLS];
}
void loop() 
{
 // Battery status.
 double battVoltage = 0.0;
 double prevVoltage = 0.0;
 // Populate battery state message.
 for (int i = 0; i < CELLS; i++)
 {
 // Read raw voltage from analog pin.
 double cellVoltage = analogRead(i) * K;
 // Scale reading to full voltage.
 cellVoltage *= cell_const[i];
 double tmp = cellVoltage;
 // Isolate current cell voltage.
 cellVoltage -= prevVoltage;
 battVoltage += cellVoltage;
 prevVoltage = tmp;
 // Set current cell voltage to message.
 batt_state.cell_voltage[i] = (float)cellVoltage;
 // Check if battery is attached.
 if (batt_state.cell_voltage[i] >= 2.0)
 batt_state.present = 1;
 else
 batt_state.present = 0;
 }
 // Update battery health.
 batt_state.voltage = (float)battVoltage;
 if (batt_state.voltage > CELLS * 4.2)
 batt_state.power_supply_health = 4; // overvoltage
 else if (batt_state.voltage < CELLS * 3.0)
 batt_state.power_supply_health = 3; // dead
 else
 batt_state.power_supply_health = 1; // good
 // Publish data to ROSSERIAL.
 batteryState.publish( &batt_state );
 nh.spinOnce();
 delay(1000);
}

Here is my header file:

#ifndef battery_h
#define battery_h
#if ARDUINO >= 100
#include "Arduino.h"
#else
#include "WProgram.h"
#endif
#include <ros.h>
#include <sensor_msgs/BatteryState.h>
#define K 0.00472199
#define CELLS 6
#define MAX_CELLS 12
#define CRITICAL 0.30
double cell_const[MAX_CELLS] = 
{
 1.0000, 2.1915, 2.6970, 4.1111,
 4.7333, 6.6000, 6.6000, 7.8293,
 8.4667, 9.2353, 11.0000, 11.0000
};
ros::NodeHandle nh;
sensor_msgs::BatteryState batt_state;
ros::Publisher batteryState("/battery/info", &batt_state);
class Battery
{
 private:
 void updateBatteryHealth();
 double battVoltage;
 double prevVoltage; 
 public:
 void begin();
 void updateBatteryState();
};
#endif

This is my cpp file:

#include "battery.h"
void Battery::begin(){
 // Launch ROS node and set parameters.
 nh.initNode();
 // Setup publishers/subscribers.
 nh.advertise(batteryState);
 // Populate battery parameters.
 batt_state.design_capacity = 2200; // mAh
 batt_state.power_supply_status = 2; // discharging
 batt_state.power_supply_health = 0; // unknown
 batt_state.power_supply_technology = 3; // LiPo
 batt_state.present = 1; // battery present
 batt_state.location = "Crawler"; // unit location
 batt_state.serial_number = "ABC_0001"; // unit serial number
 batt_state.cell_voltage = new float[CELLS]; // individual cell health
} 
void Battery::updateBatteryState() {
 battVoltage = 0.0;
 prevVoltage = 0.0;
 // Reset Power Supply Health.
 batt_state.power_supply_health = 0;
 // Populate battery state message.
 for (int i = 0; i < CELLS; i++)
 {
 // Read raw voltage from analog pin.
 double cellVoltage = analogRead(i) * K;
 // Scale reading to full voltage.
 cellVoltage *= cell_const[i];
 double tmp = cellVoltage;
 // Isolate current cell voltage.
 cellVoltage -= prevVoltage;
 battVoltage += cellVoltage;
 prevVoltage = tmp;
 // Set current cell voltage to message.
 batt_state.cell_voltage[i] = (float)cellVoltage;
 // Check if battery is attached.
 if (batt_state.cell_voltage[i] >= 2.0)
 {
 if (batt_state.cell_voltage[i] <= 3.2)
 batt_state.power_supply_health = 5; // Unspecified failure.
 batt_state.present = 1;
 }
 else{
 batt_state.present = 0;
 }
 }
};
void Battery::updateBatteryHealth() {
 // Update battery health.
 if (batt_state.present)
 {
 batt_state.voltage = (float)battVoltage;
 float volt = batt_state.voltage;
 float low = 3.0 * CELLS;
 float high = 4.2 * CELLS;
 batt_state.percentage = constrain((volt - low) / (high - low), 0.0, 1.0); 
 }
 else 
 {
 batt_state.voltage = 0.0;
 batt_state.percentage = 0.0;
 }
 // Update power supply health if not failed.
 if (batt_state.power_supply_health == 0 && batt_state.present)
 {
 if (batt_state.voltage > CELLS * 4.2)
 batt_state.power_supply_health = 4; // overvoltage
 else if (batt_state.voltage < CELLS * 3.0)
 batt_state.power_supply_health = 3; // dead
 else
 batt_state.power_supply_health = 1; // good 
 }
 // Publish data to ROSSERIAL.
 batteryState.publish( &batt_state );
 nh.spinOnce();
};

this is the error im getting simply by calling the Battery::begin function in my main class:

libraries/KBROSRobot/KBROSRobot.cpp.o (symbol from plugin): In function `KBROSRobot::KBROSRobot()':
(.text+0x0): multiple definition of `nh'
sketch/rob_test.ino.cpp.o (symbol from plugin):(.text+0x0): first defined here
libraries/KBROSRobot/KBROSRobot.cpp.o (symbol from plugin): In function `KBROSRobot::KBROSRobot()':
(.text+0x0): multiple definition of `batt_state'
sketch/rob_test.ino.cpp.o (symbol from plugin):(.text+0x0): first defined here
libraries/KBROSRobot/KBROSRobot.cpp.o (symbol from plugin): In function `KBROSRobot::KBROSRobot()':
(.text+0x0): multiple definition of `batteryState'
sketch/rob_test.ino.cpp.o (symbol from plugin):(.text+0x0): first defined here
libraries/KBROSRobot/KBROSRobot.cpp.o (symbol from plugin): In function `KBROSRobot::KBROSRobot()':
(.text+0x0): multiple definition of `cell_const'
sketch/rob_test.ino.cpp.o (symbol from plugin):(.text+0x0): first defined here
/var/folders/m4/lq13_j5j23qc0l2wp8dqmt4h0000gp/T//cczNyzqC.ltrans1.ltrans.o: In function `begin':
/Users/keith.brown/Documents/Arduino/libraries/KBROSRobot/KBROSRobot.cpp:9: undefined reference to `Battery::begin()'
collect2: error: ld returned 1 exit status
exit status 1

• KIIV: Possible duplicate of Multiple definition error when linking firmware.elf
• Juraj: @KIIV, how is this a duplicate of that? that was a mess with header guards and file extensions. here the linker can't find a function which exists in the source code. please explain if you see something I don't see
• KIIV: @Juraj In the answer I tried to cover how to declare something in the header and define it in the source file so it won't results into multiple definitions.
• Juraj: @KIIV, sorry, yes, but the answer by tttapa covers that too. but the last error is perhaps something else undefined reference to Battery::begin()
• KIIV: @Juraj True true. Undefined Battery::begin is weird. I can't see anything in the question that explains why it's failing

Answer 1

Don't define your global variables in the header file. Only declare them, using external linkage. Then define them in the implementation file

Header (.h):

extern ros::NodeHandle nh;

Implementation (.cpp):

ros::NodeHandle nh;

Try to keep usage of global variables to a minimum. If you don't need them outside of the library, declare them in the implementation file and mark them static.
Or even better: encapsulate them inside of the class.

---

The reason that you are getting these errors is because each implementation file is compiled separately. You have two implementation files that include your header file, so the header file is compiled twice. This results in two global variables with the name nh. During the linking stage, the linker tries to combine the compiled version of both KBROSRobot.cpp and rob_test.ino.cpp (these are called object files, .o). The linker has no way of knowing which variable is which, because they are both defined twice, so it throws an error.

Multiple declarations are fine, because it just tells the compiler "there is a definition somewhere", and the linker will find that definition eventually. That's why the declaration with external linkage solves your problem.
Multiple definitions are not allowed (in most cases).

• rcpilotp51: i will give the above a try - do you think i converted the sketch ok? should i do something else differently?
• tttapa: @rcpilotp51 You have to do the same for all definitions in the header file. I haven't looked at the rest of your code in detail.
• rcpilotp51: im getting an error about the extern: warning: 'batteryState' initialized and declared 'extern' extern ros::Publisher batteryState("/battery/info", &batt_state);
• tttapa: @rcpilotp51 remove the initializer in the header file (i.e. just use extern ros::Publisher batteryState;). Do add the initializer in the implementation file.
• rcpilotp51: ok got rid of the multiple errors, niw just left with : var/folders/m4/lq13_j5j23qc0l2wp8dqmt4h0000gp/T//ccB5PTqK.ltrans0.ltrans.o: In function begin': /Users/keith.brown/Documents/Arduino/libraries/KBROSRobot/KBROSRobot.cpp:9: undefined reference to Battery::begin()' collect2: error: ld returned 1 exit status exit status 1

Answer 2

I also found that all other classes in the library have to be in the utility folder or the wont compile. Answering my comment above. Thank you @tttapa for your help.

Now I just have to intelligently program the robot, which is unfortunately subjective... ;)

• Juraj: library folder structure specs: github.com/arduino/Arduino/wiki/…