ichigokurosaki
asked on
[C, C++, Boost Thread] Share variables in threads with Ubuntu and C, C++ and Lib Boost
I'm writing a mobile robot C program which reads data from a serial port to check some sensors and then try to load a joypad to control two servos.
Since i'm using a blocking read to acquire data for sonars from serial port, i can't get input from joypad and from the serial port at the same time, so i can't stop my robot in time if sonars detect a possible collision.
For this reason, after having received few infos from the serial port, I start an infinite while loop to reads possible collisions from the serial port and I start a new thread with Boost library to load and receive input from the joypad.
With this solution, i'm able to use the joypad and receive collisions information at the same time; the problem is that i'm not able to share variable between the previous thread which reads from serial port and the new thread which load the joypad so the previous thread is not able to communicate the the joypad thread if a collision occurs.
I want to do this:
Previous thread detect a collision -> write a value in a linked list -> new thread read it and stop servos.
What can i do in order to let the new thread detect a collision from the previous thread?
I have to use signal() function?
When i try to compile this code, g++ says me that linked list, and in particular, result variable is not declared in the scope.
I think because new thread does not share the old variables and declarations.
Since i'm using a blocking read to acquire data for sonars from serial port, i can't get input from joypad and from the serial port at the same time, so i can't stop my robot in time if sonars detect a possible collision.
For this reason, after having received few infos from the serial port, I start an infinite while loop to reads possible collisions from the serial port and I start a new thread with Boost library to load and receive input from the joypad.
With this solution, i'm able to use the joypad and receive collisions information at the same time; the problem is that i'm not able to share variable between the previous thread which reads from serial port and the new thread which load the joypad so the previous thread is not able to communicate the the joypad thread if a collision occurs.
I want to do this:
Previous thread detect a collision -> write a value in a linked list -> new thread read it and stop servos.
What can i do in order to let the new thread detect a collision from the previous thread?
I have to use signal() function?
When i try to compile this code, g++ says me that linked list, and in particular, result variable is not declared in the scope.
I think because new thread does not share the old variables and declarations.
int main(){
// do a lots of stuff then..
s_write(serial_port, "system-start");
boost::thread collisions_thread1(&robot_move); <-- LOAD JOYPAD IN A NEW THREAD
check_coll();
}
void check_coll(){
while(1){
std::string detect_collisions = s_readBlocking(serial_port, TIME_DELAY);
parseCommand(detect_collisions); <-- I SAVE COLLISIONS IN A LINKED LIST
}
}
void robot_move(){
// load joypad and then check for collisions
while(joypad_is_on)
// check for collisions while getting input from joypad
std::map<std::string, std::string>::const_iterator sonar_collisions = result.find("sonar-collision");
if(sonar_collisions != result.end() && (sonar_collisions->second == "ok"))
{
collisions ++;
std::cout << "STOP!\nCollisions " << sonar_collisions->second << " e count= " << collisions << "\n";
}
}
}
SOLUTION
membership
This solution is only available to members.
To access this solution, you must be a member of Experts Exchange.
ASKER
@evilrix: i'm not very expert in class programming, but i'm reading the links you posted in order to understand something about functors.
@sastumo:
I think polling the two devices would not solve the problem, because when i read from the serial port, i use a blocking function which blocks the entire program.
I have to always receive signals from the joy-pad (to move the robot i have to move the level stick) and at the same time to see if there are some collisions (data on the serial port are present only if there are collisions).
I use these functions to read from serial port and store information in a list.
@sastumo:
I think polling the two devices would not solve the problem, because when i read from the serial port, i use a blocking function which blocks the entire program.
I have to always receive signals from the joy-pad (to move the robot i have to move the level stick) and at the same time to see if there are some collisions (data on the serial port are present only if there are collisions).
I use these functions to read from serial port and store information in a list.
std::map<std::string, std::string> result;
std::pair<std::string, std::string> splitAtFirst(const std::string& str, const char* delimiters)
{
std::string::size_type i = str.find_first_of(delimiters);
if(i != std::string::npos)
{
return std::pair<std::string, std::string>(
str.substr(0, i), str.substr(i + 1)
);
}
return std::pair<std::string, std::string>(str, std::string());
}
std::map<std::string, std::string> parseCommand(const std::string& data)
{
std::pair<std::string, std::string> kv, line;
std::string temp = data;
do
{
line = splitAtFirst(temp, "\n");
if(line.second.empty())
return result;
kv = splitAtFirst(line.first, "=");
result.insert( std::map<std::string, std::string>::value_type(kv.first, kv.second) );
temp = line.second;
}
while(true);
}
std::string s_read(SerialStream& serial_port)
{
std::cout << "Test Before While Loop\n";
std::string result;
std::cout << "Serial available:" << serial_port.rdbuf()->in_avail() << "\n\n";
while( serial_port.rdbuf()->in_avail() > 0 )
{
char next_byte;
serial_port.get(next_byte);
result += next_byte;
}
return result;
}
std::string s_readBlocking(SerialStream& serial_port, int timeout)
{
while( serial_port.rdbuf()->in_avail() == 0 || timeout > 0 )
{
timeout -= 100;
usleep(100);
}
return s_read(serial_port);
}
Most of the boost and stl (and a variety of design patterns) depend on understanding functors. The concept is relatively easy to follow and I'm happy to work with you to get you head around them if you want.
So don't use a blocking form of serial read.
You just keep going around the loop reading from serial (if there is data) and reading from pad. Then according to the state of send messages to the robot. The only tricky bit is to keep tracking the robot state while going around the loop.
while (1)
{
if( serial_port.rdbuf()->in_avail() > 0 )
{
// read one collision data from serial port
}
// read from joypad
// send command to robot depending on pad and collision state
}
You just keep going around the loop reading from serial (if there is data) and reading from pad. Then according to the state of send messages to the robot. The only tricky bit is to keep tracking the robot state while going around the loop.
>> So don't use a blocking form of serial read.
The problem with that model is you end up in a busy wait -- consuming heaps of CPU for no good reason.
Ideally you'd want to use an interupt driven model, which waits on data from the serial port and once it has something it fires an event to handle that data. Meanwhile, your primary thread should be working with the real-time stuff (in this case your joystick); although, ideally that would also be an event driven model.
The problem with that model is you end up in a busy wait -- consuming heaps of CPU for no good reason.
Ideally you'd want to use an interupt driven model, which waits on data from the serial port and once it has something it fires an event to handle that data. Meanwhile, your primary thread should be working with the real-time stuff (in this case your joystick); although, ideally that would also be an event driven model.
SOLUTION
membership
This solution is only available to members.
To access this solution, you must be a member of Experts Exchange.
I agree, it would be a busy idle state. A periodic timer could be added, so that the program waits for the remainder of an interval before getting the next input data. The program can get the joypad state continuously and using a timer will make the control consistent over time.
I think a thread is over engineering a program like this and will make the program more complex. The functions in the thread would not be particularly parallel or time consuming, except for blocking on the port.
I think a thread is over engineering a program like this and will make the program more complex. The functions in the thread would not be particularly parallel or time consuming, except for blocking on the port.
a periodic timer either is based on thread or the main thread is running an endless loop and checks the timer at idle time. i don't think it would be much simpler by using a timer.
in my opinion a thread which is responsible for detecting collisions of one device and sends an event to the main thread in case of a collision is not a bad design.
Sara
in my opinion a thread which is responsible for detecting collisions of one device and sends an event to the main thread in case of a collision is not a bad design.
Sara
I tend to agree with sara: there is nothing wrong with using a thread (or threads) to do this as long as you understand how to write safe multi-threading code :)
ASKER
I'm sorry for the delay, i had some problems and i couldn't post.
First of all, thanks a lot to everyone!
I'm using the Shared class tip but i'm having some problems because i'm not still able to send events between threads.
I tried to increase the variable int collisions if collisions are detected and the to print its value in robot_move() function which is in a different thread but it always prints zero.
I was thinking to use Shared::detect list to save values about the collisions and read them in robot_move(), does this can be a good solution?
This is the code:
First of all, thanks a lot to everyone!
I'm using the Shared class tip but i'm having some problems because i'm not still able to send events between threads.
I tried to increase the variable int collisions if collisions are detected and the to print its value in robot_move() function which is in a different thread but it always prints zero.
I was thinking to use Shared::detect list to save values about the collisions and read them in robot_move(), does this can be a good solution?
This is the code:
class Shared
{
public:
static void robot_move();
static int collisions;
static bool joypad_is_on;
static std::list<std::string> detect;
};
// instantiate shared data members
int Shared::collisions = 0;
bool Shared::joypad_is_on = true;
std::list<std::string> Shared::detect;
int main(){
//.. other stuff
s_write(serial_port, "system-start");
boost::thread collisions_thread1(&Shared::robot_move);
check_coll();
}
void check_coll(){
while(Shared::joypad_is_on){
std::string detect_collisions = s_readBlocking(serial_port, TIME_DELAY);
parseCommand(detect_collisions);
// check for collisions
std::map<std::string, std::string>::const_iterator sonar_collisions = result.find("sonar-collision");
if(sonar_collisions != result.end() && (sonar_collisions->second == "ok"))
{
Shared::collisions++; // it works here, it prints increased values each time
std::cout << "STOP!\nCollisions " << sonar_collisions->second << " e count= " << Shared::collisions << "\n";
}
}
}
void Shared::robot_move(){
// loading joy pad and other stuff
std::cout << "Collisions detected from the other thread: " << Shared::collisions << "\n";
// IT DOES NOT WORK HERE, VALUE IS ALWAYS ZERO
}
ASKER CERTIFIED SOLUTION
membership
This solution is only available to members.
To access this solution, you must be a member of Experts Exchange.
the reason why the Shared::collisions is 0 in the main thread is that you need to define the member variable with the volatile specifier (see code evilrix posted).
so when using static member for collisions and joy_pad_is_on it is
the 'volatile' would tell the compiler that it should consider that the value might be changed in another thread.
the Shared::robot_move function needs to run an infinite loop similar to that in check_coll. otherwise the thread would do the cout statement once when Shared::collisions is 0 and then end.
for design i would have put the check_coll into the worker thread and the robot_move into main thread, assuming that moving the robot is the main task and detecting collisions the helper part. but that probably is matter of taste. generally, i don't think that race conditions can occur when all shared variables were properly initialized before a new thread was created and when one thread is reading and the other one is writing and both are running an endless loop. if one thread increments a shared integer it doesn't matter when the other thread is checking for the incremented variable same time even if it still reads 0 while the other same time has written 1. it surely would detect the value 1 next loop cycle and though synchronization would not harm, it is simply not needed. the only code that needs synchronization is the access to the map cause writing to and deleteing from map must be done exclusively or it might crash and while there is a delete or update operation done by one of the threads is in progress there also should be no read access by the other thread cause the container could be in an incomplete state.
note the lock_mutex and unlock_mutex functions, depend on the kind of mutex used. if using a pthread_mutex_t member in class Shared it would turn to calls of pthread_mutex_lock and pthread_mutex_unlock.
note also, the above functions only need to be defined static in the class if the map is (and other data members are) static. making all variables and functions static makes Shared a singleton class what makes it easier to get the things running as you don't need to care for passing an object between threads. later, you easily can remove the static keyword from all members (beside of the thread function) and create an object of class Shared before thread creation. you then would pass a pointer to that (shared) object and that way both the main thread and the worker thread would share this object and its members rather than have access to static members of the class.
using boost threads with bost bind functors is high-end programming technique. i don't think it is a technique which should be used for the first multi-threading project. i think you would need to gain more experience before you could successfully practice that high-level approach. but that is my personal opinion. if you feel fit enough to go that way, i wish you good luck.
Sara
so when using static member for collisions and joy_pad_is_on it is
class Shared
{
public:
static volatile int collisions;
static volatile bool joy_pad_is_on ;
static void robot_move(void * p);
...
};
// static initializations
volatile int Shared::collisions = 0;
volatile bool Shared::joy_pad_is_on = false;
the 'volatile' would tell the compiler that it should consider that the value might be changed in another thread.
the Shared::robot_move function needs to run an infinite loop similar to that in check_coll. otherwise the thread would do the cout statement once when Shared::collisions is 0 and then end.
for design i would have put the check_coll into the worker thread and the robot_move into main thread, assuming that moving the robot is the main task and detecting collisions the helper part. but that probably is matter of taste. generally, i don't think that race conditions can occur when all shared variables were properly initialized before a new thread was created and when one thread is reading and the other one is writing and both are running an endless loop. if one thread increments a shared integer it doesn't matter when the other thread is checking for the incremented variable same time even if it still reads 0 while the other same time has written 1. it surely would detect the value 1 next loop cycle and though synchronization would not harm, it is simply not needed. the only code that needs synchronization is the access to the map cause writing to and deleteing from map must be done exclusively or it might crash and while there is a delete or update operation done by one of the threads is in progress there also should be no read access by the other thread cause the container could be in an incomplete state.
void Shared::push_back(const std::string & k, const std::string & d)
{
lock_mutex();
result[k] = d;
unlock_mutex();
}
size_type Shared::remove(const std::string & k)
{
lock_mutex();
size_type n = result.erase[k];
unlock_mutex();
return n;
}
std::string Shared::find(const std::string & k)
{
std::string d;
lock_mutex();
std::map<std::string, std::string>::iterator f;
f = result.find[k];
if (f != result.end())
d = f->second;
unlock_mutex();
return d;
}
note the lock_mutex and unlock_mutex functions, depend on the kind of mutex used. if using a pthread_mutex_t member in class Shared it would turn to calls of pthread_mutex_lock and pthread_mutex_unlock.
note also, the above functions only need to be defined static in the class if the map is (and other data members are) static. making all variables and functions static makes Shared a singleton class what makes it easier to get the things running as you don't need to care for passing an object between threads. later, you easily can remove the static keyword from all members (beside of the thread function) and create an object of class Shared before thread creation. you then would pass a pointer to that (shared) object and that way both the main thread and the worker thread would share this object and its members rather than have access to static members of the class.
using boost threads with bost bind functors is high-end programming technique. i don't think it is a technique which should be used for the first multi-threading project. i think you would need to gain more experience before you could successfully practice that high-level approach. but that is my personal opinion. if you feel fit enough to go that way, i wish you good luck.
Sara
ASKER
Hi guys, thanks again for your suggestions.
Sorry for the delay, but i had health problems for many days.
I've read evilrix and sarabande suggestions and i tried to use the "volatile" specification in order to share values between threads without no results :(
I've read the links posted by evilrix and I realised that volatile is the key to let different threads to share variable values; unfortunately, it seems to not work properly for me: i did some tests and the second thread is able to just "see" the first variable values but after this it does not update the variable status anymore.
For example:
first thread: collisions = 3
second thread: collision = 3
first thread: collision = 5
second thread: collision = 3
first thread: collision = 7
second thread: collision = 3
and so on: second thread will never change anymore while the first continues to increase the collision value.
Then, i tried the evilrix code, but the g++ compiler gives me a lots of errors.
Due to my poor programming knowledge, this is a very hard task for me: it seems i won't be able to make these two threads work properly! :(
However, i have to say that i'm learning (a lot) about multi-threading applications.
This is the code:
and this is the error:
Here, you can find the complete C++ code i'm using: http://www.dronyx.com/robot.cpp
Sorry for the delay, but i had health problems for many days.
I've read evilrix and sarabande suggestions and i tried to use the "volatile" specification in order to share values between threads without no results :(
I've read the links posted by evilrix and I realised that volatile is the key to let different threads to share variable values; unfortunately, it seems to not work properly for me: i did some tests and the second thread is able to just "see" the first variable values but after this it does not update the variable status anymore.
For example:
first thread: collisions = 3
second thread: collision = 3
first thread: collision = 5
second thread: collision = 3
first thread: collision = 7
second thread: collision = 3
and so on: second thread will never change anymore while the first continues to increase the collision value.
Then, i tried the evilrix code, but the g++ compiler gives me a lots of errors.
Due to my poor programming knowledge, this is a very hard task for me: it seems i won't be able to make these two threads work properly! :(
However, i have to say that i'm learning (a lot) about multi-threading applications.
This is the code:
class Shared
{
public:
Shared(boost::mutex & mtx) // mutex passed into constructor from main thread
: collisions(0)
, joypad_is_on(true) <-- I DO NOT KNOW THIS SINTAX: IS IT CORRECT?
, mtx(mtx) {}
void robot_move();
// shared data members (these need to be volatile)
volatile int collisions;
volatile bool joypad_is_on;
// As this object isn't designed to be volatile you may have issues x-thread
std::list<std::string> detect;
// mutable so it will still work if the class is const
mutable boost::mutex & mtx; // you need a reference to a shared mutex for syncing shared access
}
// instantiate shared data members
volatile int Shared::collisions = 0;
volatile bool Shared::joypad_is_on = true;
std::list<std::string> Shared::detect;
int main(){
..
if ( (sonar_front->second == "ok") && (sonar_left->second == "ok") && (sonar_right->second == "ok")){
std::cout << "Sonar OK\n";
s_write(serial_port, "system-start");
boost::mutex mtx; // used to sync access
Shared shared(mtx); // pass mutex to shared.
boost::thread collisions_thread1(boost::bind(&Shared::robot_move, shared));
check_coll(shared);
}
}
void robot_move(){
// do something, load joypad then call
send_event(..)
}
int send event(..){
// some stuff
boost::lock_guard lock(mtx); // lock shared mutex
std::cout << "Collisioni altro thread: " << Shared::collisions << "\n";
}
void check_coll(Shared & shared){
while(true){
boost::lock_guard lock(shared.mtx); // lock shared mutex
if(!shared.joypad_is_on) { break; } // no longer on, so quite loop
std::string detect_collisions = s_readBlocking(serial_port, TIME_DELAY);
parseCommand(detect_collisions);
// check for collisions
std::map<std::string, std::string>::const_iterator sonar_collisions = result.find("sonar-collision");
if(sonar_collisions != result.end() && (sonar_collisions->second == "ok"))
{
++shared.collisions;
std::cout << "STOP!\nCollisions " << sonar_collisions->second << " e count= " << Shared::collisions << "\n";
}
}
}
and this is the error:
robot.cpp:91:22: error: two or more data types in declaration of ‘collisions’
robot.cpp:92:23: error: ‘volatile bool Shared::joypad_is_on’ is not a static member of ‘class Shared’
robot.cpp:93:32: error: ‘std::list<std::basic_string<char> > Shared::detect’ is not a static member of ‘class Shared’
robot.cpp: In function ‘int send_event(int, __u16, __u16, __s32)’:
robot.cpp:103:20: error: missing template arguments before ‘lock’
robot.cpp:103:20: error: expected ‘;’ before ‘lock’
robot.cpp:80:16: error: invalid use of non-static data member ‘Shared::collisions’
robot.cpp:104:61: error: from this location
robot.cpp:523:20: error: too many arguments to function ‘void check_coll()’
robot.cpp:66:6: note: declared here
robot.cpp:750:20: error: missing template arguments before ‘lock’
robot.cpp:750:20: error: expected ‘;’ before ‘lock’
robot.cpp:80:16: error: invalid use of non-static data member ‘Shared::collisions’
robot.cpp:762:92: error: from this location
Here, you can find the complete C++ code i'm using: http://www.dronyx.com/robot.cpp
the volatile only tells the compiler to read the variable from memory every time when it was accessed. without volatile it would read in a function only once and would not assume that it could be changed by another thread asynchronously.
the errors are because only static members must be initialized outside the class. non-static members were initialized in the constructor which was called for any instance you define or create with new on the heap.
note, it is easier for you to follow the advice of evilrix and NOT use static members.
i am sure he also will help you with the boost compile errors what is more difficult for me cause ioon my current system boost is not available.
Sara
the errors are because only static members must be initialized outside the class. non-static members were initialized in the constructor which was called for any instance you define or create with new on the heap.
class Shared
{
static int m_static; // exist only once for class must be initialized
int m_nonstatic; // exists for any instance (object) of class Shared
public:
Shared() : m_nonstatic(0) {} // initialization happens in constructor
};
// static initialization cause Shared not necessarily has an instance at all
int Shared::m_static = 0;
note, it is easier for you to follow the advice of evilrix and NOT use static members.
i am sure he also will help you with the boost compile errors what is more difficult for me cause ioon my current system boost is not available.
Sara
ASKER
Sara, thank you a lot!
(I'm trying to implement your solution but it's a little bit tricky for me)
I deleted the variables definition outside the Shared class and now errors about the static members are disappeared.
Unfortunately, g++ gives me others error:
This is the whole updated code: http://www.dronyx.com/robot.cpp
(I'm trying to implement your solution but it's a little bit tricky for me)
I deleted the variables definition outside the Shared class and now errors about the static members are disappeared.
Unfortunately, g++ gives me others error:
robot.cpp:69:1: error: new types may not be defined in a return type
robot.cpp:69:1: note: (perhaps a semicolon is missing after the definition of ‘Shared’)
robot.cpp:92:65: error: two or more data types in declaration of ‘send_event’
robot.cpp: In function ‘int main(int, char**)’:
robot.cpp:519:20: error: too many arguments to function ‘void check_coll()’
robot.cpp:66:6: note: declared here
robot.cpp:609:17: error: ‘send_event’ was not declared in this scope
robot.cpp:615:17: error: ‘send_event’ was not declared in this scope
robot.cpp:618:49: error: ‘send_event’ was not declared in this scope
robot.cpp:636:17: error: ‘send_event’ was not declared in this scope
robot.cpp:642:17: error: ‘send_event’ was not declared in this scope
robot.cpp:645:49: error: ‘send_event’ was not declared in this scope
robot.cpp:701:20: error: ‘send_event’ was not declared in this scope
robot.cpp:705:48: error: ‘send_event’ was not declared in this scope
robot.cpp: In function ‘void check_coll(Shared&)’:
robot.cpp:746:20: error: missing template arguments before ‘lock’
robot.cpp:746:20: error: expected ‘;’ before ‘lock’
robot.cpp:80:16: error: invalid use of non-static data member ‘Shared::collisions’
robot.cpp:758:92: error: from this location
This is the whole updated code: http://www.dronyx.com/robot.cpp
i would need the corresponding code for an analysis. you should take the errors as literally as possible. if for example it says "new types may not be defined in a return type" you probably only has a typo at return type of a function like voiid. or you din't include <string> but use std::string for key of the map.
Sara
Sara
ASKER
At this link: http://www.dronyx.com/robot.cpp you can find the corresponding code with the same error lines specified by g++.
It's strange because the code map was working before to introduce the new thread code.
I'm trying to analyse each error line to see if i'm able to fix it.
It's strange because the code map was working before to introduce the new thread code.
I'm trying to analyse each error line to see if i'm able to fix it.
line 69 is begin of definition of class Shared.
you firstly should end class definition of Shared with a semicolon ;
if that doesn't solve at least the first errors it means the real error is above, probably in one of the header files. here i would need SerialStream.h
Sara
you firstly should end class definition of Shared with a semicolon ;
if that doesn't solve at least the first errors it means the real error is above, probably in one of the header files. here i would need SerialStream.h
Sara
ASKER
Hi Sara,
i really didn't see the missing semicolon at the end of the class declaration.
I added it, now the first error is disappeared as you supposed, however, the other ones are still there.
i really didn't see the missing semicolon at the end of the class declaration.
I added it, now the first error is disappeared as you supposed, however, the other ones are still there.
robot.cpp: In function ‘int send_event(int, __u16, __u16, __s32)’:
robot.cpp:99:20: error: missing template arguments before ‘lock’
robot.cpp:99:20: error: expected ‘;’ before ‘lock’
robot.cpp:80:16: error: invalid use of non-static data member ‘Shared::collisions’
robot.cpp:100:61: error: from this location
robot.cpp: In function ‘int main(int, char**)’:
robot.cpp:519:20: error: too many arguments to function ‘void check_coll()’
robot.cpp:66:6: note: declared here
robot.cpp: In function ‘void check_coll(Shared&)’:
robot.cpp:746:20: error: missing template arguments before ‘lock’
robot.cpp:746:20: error: expected ‘;’ before ‘lock’
robot.cpp:80:16: error: invalid use of non-static data member ‘Shared::collisions’
robot.cpp:758:92: error: from this location
>> i am sure he also will help you with the boost compile errors
Happy to. Can you post the code as it currently stands and I'll take a look and see if I can get it building for you?
Happy to. Can you post the code as it currently stands and I'll take a look and see if I can get it building for you?
<< joypad_is_on(true) <-- I DO NOT KNOW THIS SINTAX: IS IT CO
Yes :)
http://www.cprogramming.com/tutorial/initialization-lists-c++.html
Not only is it correct but it is the preferred (and sometimes only way) to initialise class members.
Yes :)
http://www.cprogramming.com/tutorial/initialization-lists-c++.html
Not only is it correct but it is the preferred (and sometimes only way) to initialise class members.
>> robot.cpp:746:20: error: missing template arguments before ‘lock’
Oops, my bad. This...
..should be this...
...because the lock needs to know what type of mutex it is working with.
Also, unless you have included <boost/thread.hpp> you will also need to include <boost/thread/locks.hpp>.
>> invalid use of non-static data member ‘Shared::collisions’
You are probably using Shared::collisions where you should be using shared.colissions (it's an instance member not a static member).
Please post your code if you still have errors and I'll take a look.
Oops, my bad. This...
boost::lock_guard lock(mtx)
..should be this...
boost::lock_guard<boost::mutex> lock(mtx)
...because the lock needs to know what type of mutex it is working with.
Also, unless you have included <boost/thread.hpp> you will also need to include <boost/thread/locks.hpp>.
>> invalid use of non-static data member ‘Shared::collisions’
You are probably using Shared::collisions where you should be using shared.colissions (it's an instance member not a static member).
Please post your code if you still have errors and I'll take a look.
to the errors:
you need to pass 'Shared & shared' as argument to send_event such that you can use the mutex member of that class. as the mutex is not a static member anymore you need an instance of Shared to pass between functions or a member of type Shared in a member function.
Sara
you need to pass 'Shared & shared' as argument to send_event such that you can use the mutex member of that class. as the mutex is not a static member anymore you need an instance of Shared to pass between functions or a member of type Shared in a member function.
int send_event(Shared & shared, int fd, ...)
{
....
boost::lock_guard lock(shared.mtx); // lock shared mutex
...
// always use shared. instead of Shared:: now
std::cout << "Collisioni altro thread: " << shared.collisions << "\n";
...
Sara
ASKER
Thanks for your support, guys!
I did what you suggested (i added boost/thread/locks.hpp, i changed the mutex declaration and i tried to use shared.collisions instead of Shared::collisions) and now some errors are disappeared.
Unfortunately, now, g++ tells me that the mutex mtx is not declared in the scope.
I think the reason is because i do not have an instance of Shared in send_event() function as Sara said; i tried to add Shared &shared in send_event() and to use shared.mtx but i didn't solved the problem.
And this is the current code i'm using: http://www.dronyx.com/robot.cpp
I hope you can help me, in the meanwhile, i'm trying to see if i'm able to solve some of the errors by looking for them on the net.
I did what you suggested (i added boost/thread/locks.hpp, i changed the mutex declaration and i tried to use shared.collisions instead of Shared::collisions) and now some errors are disappeared.
Unfortunately, now, g++ tells me that the mutex mtx is not declared in the scope.
I think the reason is because i do not have an instance of Shared in send_event() function as Sara said; i tried to add Shared &shared in send_event() and to use shared.mtx but i didn't solved the problem.
int send_event(int fd, __u16 event_type, __u16 code, __s32 value) {
Shared &shared;
boost::lock_guard<boost::mutex> lock(shared.mtx); // lock shared mutex
std::cout << "Collisioni altro thread: " << shared.collisions << "\n";
These are the whole g++ errors:robot.cpp: In function ‘int send_event(int, __u16, __u16, __s32)’:
robot.cpp:94:18: error: ‘shared’ declared as reference but not initialized
robot.cpp: In function ‘int main(int, char**)’:
robot.cpp:521:20: error: too many arguments to function ‘void check_coll()’
robot.cpp:67:6: note: declared here
robot.cpp: In function ‘void check_coll(Shared&)’:
robot.cpp:81:16: error: invalid use of non-static data member ‘Shared::collisions’
robot.cpp:760:92: error: from this location
And this is the current code i'm using: http://www.dronyx.com/robot.cpp
I hope you can help me, in the meanwhile, i'm trying to see if i'm able to solve some of the errors by looking for them on the net.
You need to pass a reference to shared into send_event... currently you are trying to create a local reference.
int send_event(Shared &shared, int fd, __u16 event_type, __u16 code, __s32 value) {
boost::lock_guard<boost::mutex> lock(shared.mtx); // lock shared mutex
std::cout << "Collisioni altro thread: " << shared.collisions << "\n";
you have to use always the same Shared object. otherwise the mutex could not work. so you cannot declare a local Shared object shared in send_event and use that but has to pass THE Shared object which was associated and passed to the thread.
so it is that your main thread creates a Shared object 'shared' before thread creation and passes a pointer to that object to the thread which does the robot_move. then the main thread does the collision checks. that way both threads could use the shared for synchronization - what means they both use mtx member of the same instance of class Shared.
as told you need to add an argument to send_event
or make send_event a class member function of Shared as it was designed by evilrix.
in the first case the mutex would be accessed by shared.mtx and in the second case only by mtx but both robot_move and send_event were member functions of Shared.
Sara
p.s.
i still think that boost functors and boost threading are very difficult for you though it is a superior design.
so it is that your main thread creates a Shared object 'shared' before thread creation and passes a pointer to that object to the thread which does the robot_move. then the main thread does the collision checks. that way both threads could use the shared for synchronization - what means they both use mtx member of the same instance of class Shared.
as told you need to add an argument to send_event
int send_event(Shared & shared, int fd, ...)
or make send_event a class member function of Shared as it was designed by evilrix.
int Shared::send_event(int fd, ...)
{
in the first case the mutex would be accessed by shared.mtx and in the second case only by mtx but both robot_move and send_event were member functions of Shared.
Sara
p.s.
i still think that boost functors and boost threading are very difficult for you though it is a superior design.
>> boost functors and boost threading are very difficult for you though it is a superior design.
I agree with threading -- writing safe MT code is one of the hardest things to do.
I don't agree with functors... if you can understand how a function works you can understand how a functor works -- it's just a function that has state. The specific syntax may been a little hard to initially grasp (since the use of functors often involves using templates) but the principle is easy enough to understand.
I agree with threading -- writing safe MT code is one of the hardest things to do.
I don't agree with functors... if you can understand how a function works you can understand how a functor works -- it's just a function that has state. The specific syntax may been a little hard to initially grasp (since the use of functors often involves using templates) but the principle is easy enough to understand.
ASKER
If i well understood what you suggested me, i did these changes:
And some other errors are disappeared.
These are the remaining errors:
For this error in particular:
i tried to use also shared.collisions but it still gives me the error.
This is the whole code: http://www.dronyx.com/robot.cpp
class Shared
{
public:
Shared(boost::mutex & mtx) // mutex passed into constructor from main thread
: collisions(0)
, joypad_is_on(true)
, mtx(mtx) {}
void robot_move();
int send_event(int, __u16, __u16, __s32);
// shared data members (these need to be volatile)
volatile int collisions;
volatile bool joypad_is_on;
// As this object isn't designed to be volatile you may have issues x-thread
std::list<std::string> detect;
// mutable so it will still work if the class is const
mutable boost::mutex & mtx; // you need a reference to a shared mutex for syncing shared access
};
int Shared::send_event(int fd, __u16 event_type, __u16 code, __s32 value) {
boost::lock_guard<boost::mutex> lock(mtx); // lock shared mutex
std::cout << "Collisioni altro thread: " << shared.collisions << "\n";
And some other errors are disappeared.
These are the remaining errors:
robot.cpp:521:20: error: ‘check_coll’ was not declared in this scope
robot.cpp: In member function ‘void Shared::robot_move()’:
robot.cpp:555:23: warning: taking address of temporary
robot.cpp: In function ‘void check_coll(Shared&)’:
robot.cpp:81:16: error: invalid use of non-static data member ‘Shared::collisions’
robot.cpp:760:92: error: from this location
For this error in particular:
invalid use of non-static data member ‘Shared::collisions’
i tried to use also shared.collisions but it still gives me the error.
This is the whole code: http://www.dronyx.com/robot.cpp
>> robot.cpp:521:20: error: ‘check_coll’ was not declared in this scope
You're using this before it's defined. Place a declaration at the top of the file.
>> robot.cpp:81:16: error: invalid use of non-static data member ‘Shared::collisions’
It absolutely MUST be shared.collisions -- if you are getting an error it will be due to something else. Please say what that error is.
>> robot.cpp:555:23: warning: taking address of temporary
That refers to the following:
while (bacmp(bdaddr, BDADDR_ANY)) {
what is bacmp? Is it a macro? Can you show the code for that please?
You're using this before it's defined. Place a declaration at the top of the file.
>> robot.cpp:81:16: error: invalid use of non-static data member ‘Shared::collisions’
It absolutely MUST be shared.collisions -- if you are getting an error it will be due to something else. Please say what that error is.
>> robot.cpp:555:23: warning: taking address of temporary
That refers to the following:
while (bacmp(bdaddr, BDADDR_ANY)) {
what is bacmp? Is it a macro? Can you show the code for that please?
Invalid use of non-static data member ‘
Shared::collisions cannot be replaced by shared.collisions when it is in a member function of Shared. then it is simply 'collissions' without any prefix.
the bacmp(bdaddr, BDADDR_ANY) refers to macros and functions defined in bluetooth.h . i think the warning can be ignored as it probably is a warning of the c++ compiler compiling older c code.
Sara
ASKER
As you said, i tried to use "collisions" in send_event() function and to use "shared.collisions" in check_coll() and g++ seems to not report the problem anymore.
Now, there is only the error with check_coll() function:
is
I declared the check_coll at the top of the file but i'm surely did a mistake because i typed:
This is the current code: http://www.dronyx.com/robot.cpp
P.S. the bacmp(bdaddr, BDADDR_ANY) warning is due to the old compiler version in fact if i use gcc instead of g++ i do not have that error even if the code is still the same.
Now, there is only the error with check_coll() function:
is
robot.cpp:68:17: error: variable or field ‘check_coll’ declared void
robot.cpp:68:17: error: ‘Shared’ was not declared in this scope
robot.cpp: In function ‘int main(int, char**)’:
robot.cpp:522:20: error: ‘check_coll’ was not declared in this scope
I declared the check_coll at the top of the file but i'm surely did a mistake because i typed:
void check_coll(Shared);
This is the current code: http://www.dronyx.com/robot.cpp
P.S. the bacmp(bdaddr, BDADDR_ANY) warning is due to the old compiler version in fact if i use gcc instead of g++ i do not have that error even if the code is still the same.
i found 3 occurences of check_coll in your yesterday source:
if you didn't remove the first occurence the two others would give error.
if you changed the first occurence using the Shared as argument type it would give error because Shared was not declared yet. you could add a forward declaration:
that compiles cause for a reference argument Shared& the compiler doesn't need full class definition. same would apply if only using a pointer of class Shared but never dereference it.
if you removed the forward declaration of check_coll you get error cause in main check_coll was called and this calls also needs at least a forward declaration of check_coll. you could move the function body of check_coll above main function but the correct solution is to use forward declarations as shown.
Sara
// above class Shared
void check_coll(void);
// in main after starting the thread
check_coll(shared);
// below Shared::robot_move
void check_coll(Shared & shared){
if you didn't remove the first occurence the two others would give error.
if you changed the first occurence using the Shared as argument type it would give error because Shared was not declared yet. you could add a forward declaration:
// forward declarations
class Shared;
void check_coll(Shared & shared);
that compiles cause for a reference argument Shared& the compiler doesn't need full class definition. same would apply if only using a pointer of class Shared but never dereference it.
if you removed the forward declaration of check_coll you get error cause in main check_coll was called and this calls also needs at least a forward declaration of check_coll. you could move the function body of check_coll above main function but the correct solution is to use forward declarations as shown.
Sara
ASKER
Thanks Sara, i did as you suggested and now it compiles without errors.
It should work, but it does not because the first thread is ok and it executes check_coll() function which reads from serial port, but robot_move() function does not work properly.
robot_move() is able to recognise the joypad but then it do nothing, send_event() is never called and the printf() i added in robot_move() to check if the program enters in the while() loop never display.
So, the second thread seems to do not start at all.
Can it be a mutex problem?
This is the current code: http://www.dronyx.com/robot.cpp
It should work, but it does not because the first thread is ok and it executes check_coll() function which reads from serial port, but robot_move() function does not work properly.
robot_move() is able to recognise the joypad but then it do nothing, send_event() is never called and the printf() i added in robot_move() to check if the program enters in the while() loop never display.
So, the second thread seems to do not start at all.
Can it be a mutex problem?
This is the current code: http://www.dronyx.com/robot.cpp
you now call fork from a thread. fork creates child processes and i don't think you could/should do that from thread. also the Shared object associated to the thread would (somehow?) copied when fork was called what means that the mutex object passed also was copied. i don't think the concept is valid.
i think my unease about putting the robot_move into thread and keep collission check in the main thread was justified and would think it would be much better to have the robot_move in the main thread then do the fork for each robot then create the Shared object (which would be a separate instance for each fork process) and then create a thread whichs runs the check_coll. thereby i assume that check_coll does check collissions of one robot only (if not you would need additional synchronization among robots). with that each (child) process would have two threads, one running the robot and one checking collissions.
Sara
i think my unease about putting the robot_move into thread and keep collission check in the main thread was justified and would think it would be much better to have the robot_move in the main thread then do the fork for each robot then create the Shared object (which would be a separate instance for each fork process) and then create a thread whichs runs the check_coll. thereby i assume that check_coll does check collissions of one robot only (if not you would need additional synchronization among robots). with that each (child) process would have two threads, one running the robot and one checking collissions.
Sara
ASKER
Hi Sara,
i'm trying to do what you suggested and i'm swapping the robot_move() with the check_coll() function but, again, i'm receiving a lots of errors:
This is a very difficult task for me, may be i should quit this approach and try to use UDP socket instead to use the serial port, may be, i can solve the problem in this other way :(
P.S. i really do not need to use fork() since i can use just a single joypad.
i'm trying to do what you suggested and i'm swapping the robot_move() with the check_coll() function but, again, i'm receiving a lots of errors:
robot.cpp:92:78: error: ISO C++ forbids declaration of ‘send_event’ with no type
robot.cpp: In function ‘int send_event(Shared&, int, __u16, __u16, __s32)’:
robot.cpp:100:46: error: ‘mtx’ was not declared in this scope
robot.cpp:101:53: error: ‘collisions’ was not declared in this scope
robot.cpp: In function ‘int main(int, char**)’:
robot.cpp:520:20: error: ‘robot_move’ was not declared in this scope
This is a very difficult task for me, may be i should quit this approach and try to use UDP socket instead to use the serial port, may be, i can solve the problem in this other way :(
P.S. i really do not need to use fork() since i can use just a single joypad.
You could try the polling method I talked about above, it's much simpler.
ASKER
The polling method would result in a non real-time system because serial port does not always read incoming data if I have to poll it.
Or am i wrong?
Or am i wrong?
I can't say for sure, having never done what you're doing, but I would be surprised if that was the case. All I/O is buffered to some degree. In this case the function name serial_port.rdbuf()->in_av ail() says that there is a buffer and that data is available in that buffer until you ask for it.
I don't know if there is a risk of data loss if the buffer overflows, in this case. I can't image that being a problem as long as you poll often enough. Will the robot generate multiple collisions in bursts?
I don't know if there is a risk of data loss if the buffer overflows, in this case. I can't image that being a problem as long as you poll often enough. Will the robot generate multiple collisions in bursts?
Or do you mean that you poll it a short time after the data arrives? I guess so, milliseconds later, how real time does it need to be? Surely whatever your robot does as a reaction will take far longer than the polling delay.
ASKER
The problem with the polling is that in future i'll have to continously receive data from the serial port while i'm controlling the motors because i'll need to receive information not only about sonars but also from encoders and gyroscopes so I have to assume that there will be always some data to read from the serial port.
The best solution was to have an infinite while loop which always read from serial port, tokenize the received information and share data between the control function..
It's a really hard task to do for me.
The best solution was to have an infinite while loop which always read from serial port, tokenize the received information and share data between the control function..
It's a really hard task to do for me.
Yes, but polling only needs to happens when there is no data.
The process would be -
1. Check for data
2. If there is no data wait for a while then go back to step 1
3. If there is data read and process until there is no more data
4. Go back to step 1.
Step 3 is a loop, it keeps reading as long as the robot keeps sending. It can allow for a certain amount of delay in the data received, a small time out. If no more data arrives before the time out, assume that the robot has stopped sending and go back to checking and waiting again.
The thing the concern me about the threading is that you're making this much harder in many ways. Not just coding it but timing too, for example it's hard to tell the second thread to read continuously for a while, like you can in this case.
Its harder to manage access to the data, especially if its arriving in uneven bursts. The second thread might not communicate the data until an entire burst has been read.
The process would be -
1. Check for data
2. If there is no data wait for a while then go back to step 1
3. If there is data read and process until there is no more data
4. Go back to step 1.
Step 3 is a loop, it keeps reading as long as the robot keeps sending. It can allow for a certain amount of delay in the data received, a small time out. If no more data arrives before the time out, assume that the robot has stopped sending and go back to checking and waiting again.
The thing the concern me about the threading is that you're making this much harder in many ways. Not just coding it but timing too, for example it's hard to tell the second thread to read continuously for a while, like you can in this case.
Its harder to manage access to the data, especially if its arriving in uneven bursts. The second thread might not communicate the data until an entire burst has been read.
if you don't need the fork you might try to remove the statements
from your last code posted (plus the closing } of the last if statement).
in my opinion polling is not so much easier than multi-threading but it would throw you back to the beginning.
Sara
pid_t pid = fork();
if (pid < 0) {
perror("fork() failed");
exit(errno);
}
if (!pid) {
from your last code posted (plus the closing } of the last if statement).
in my opinion polling is not so much easier than multi-threading but it would throw you back to the beginning.
Sara
ASKER
Hi guys, i studied a little bit the thread literature and i was able to modify my code in order to let it use three thread and share variables without problems.
At the moment, i use the first thread to load the joypad, the second to communicate over the serial port and the third to control motors.
I'm testing it and i'll post the results.
I'm using Aria thread library which i already used one year ago for another mobile robot. :)
At the moment, i use the first thread to load the joypad, the second to communicate over the serial port and the third to control motors.
I'm testing it and i'll post the results.
I'm using Aria thread library which i already used one year ago for another mobile robot. :)
Thanks for the update... looking forward to seeing how you've got on :)
ASKER
Thanks to all of you, guys!
By opening this topic, i learned a lot about C/C++, classes and threads.
I studied a little Aria library which is meanly used for mobile robot applications and i discovery that it implements also multithreading, so i decided to use:
http://math.hws.edu/vaughn/cpsc/336/Aria/docs/classArASyncTask.html
in order to start the threads.
Actually, i'm using four thread (the main thread included) to control different routines and they seems to work very well; i'm able to share variables and exchange information and signals between the threads.
Really thanks to all!
By opening this topic, i learned a lot about C/C++, classes and threads.
I studied a little Aria library which is meanly used for mobile robot applications and i discovery that it implements also multithreading, so i decided to use:
http://math.hws.edu/vaughn/cpsc/336/Aria/docs/classArASyncTask.html
in order to start the threads.
Actually, i'm using four thread (the main thread included) to control different routines and they seems to work very well; i'm able to share variables and exchange information and signals between the threads.
Really thanks to all!
ASKER
Many thanks to all, guys!
(Evilrix, Sara, Sastumo)
(Evilrix, Sara, Sastumo)
You're welcome, thanks for the points.
Since you're passing in an object that object can contain state, which includes having references or pointers to shared data.
http://www.newty.de/fpt/functor.html#intro
Open in new window
Don't forget to make any variables that cross thread barriors volatile otherwise the compiler may very well optimize out changes made in one thread as seen from the other threads.
http://drdobbs.com/cpp/184403766