Asynchronous monitoring of a class variable not working

Please pardon the mouthful of a title, I couldn’t think of anything shorter.

I’m trying to create a class that has an std::queue variable attached to it, and some methods. One method pushes values to the queue, one method enters a while loop and performs actions based on the contents of the queue, one method is a static method that takes a pointer to an instance of the object as an argument and calls the aforementioned looping method of that instance, and one is a method that starts a thread with the previous method as a callback and this as an argument.

At the crux of it, what I’m trying to do is this: have an object with a method that changes the value of a variable, and another method running in a different thread that reacts to that change and does something (in this case, access/pop the front element of the queue and do stuff with it)

My problem is that the method running in a different thread doesn’t seem to be able to register changes to the queue, and thus always thinks the queue is empty. I’ve managed to reproduce the problem in a sample project, where the queue is replaced by a simple boolean. If it were working properly, the asynchronous method would notice when the variable changes and color the screen green, but it does not. How should I go about fixing this? I’m somewhat new to the more advanced features of c++, so go easy on me. I’m sure there’s a silly error that I’ve made somewhere in here. :stuck_out_tongue:

Sample Project that Reproduces Error

robot-config.h

[default code that appears when project is created]

vex.h

[default code snipped]
//
#include <math.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>

#include "v5.h"
#include "v5_vcs.h"
#include "foo.h"

#include "robot-config.h"

[default code snipped]

foo.h

#ifndef FOO_H
#define FOO_H

#include "robot-config.h"

class foo {
  private:

    static int callback(void *arg) {
      if (arg == NULL) {return 0;}
      
      foo *instance = static_cast<foo *>(arg);

      instance->watch_variable();
    }

    void watch_variable() {
      while (true) {
        if (variable) {Brain.Screen.clearScreen(green);}
        else {Brain.Screen.clearScreen(red);}
      }
    }

  public:
    bool variable = false;

    void setvar() {
      variable = true;
    }

    void start_watching() {
      task watcher = task(foo::callback, static_cast<void *>(this));
    }
};

#endif

main.cpp

[comments snipped]

// ---- START VEXCODE CONFIGURED DEVICES ----
// ---- END VEXCODE CONFIGURED DEVICES ----

#include "vex.h"

using namespace vex;

int main() {
  // Initializing Robot Configuration. DO NOT REMOVE!
  vexcodeInit();
  
  foo f = foo();
  f.start_watching(); //start watching the variable
  f.setvar(); //set the variable to true
  //the variable watcher should register the variable being set to true and color the screen green,
  //but it does not.
}

robot-config.cpp

[default code]
2 Likes

You should really have your watcher thread have a delay in the loop so that ir doesnt block other code from executing (in fact, I suspect that this may be preventing setvar() from running), and maybe consider using a Mutex to help prevent race conditions.

1 Like

Ah, thanks, those are both good points. I have both a delay and mutexes implemented in the original version of the code, I forgot to add them in the sample project.

I’m also not super familiar with use of static_cast, but you might need static_cast<void*>(&this);

Doing static_cast<void *>(&this) gives me:

[clang] Cannot take the address of an rvalue of type 'foo *'

I think c++ might automatically reference class instances as pointers, I dunno

Ok so in my own code I have something very similar actually, except I never use static_cast; I just pass a pointer to the object, in this case a controller. I do use static_cast on the other end to convert it from a void * to a Controller *, though. Can you try just passing &this?

1 Like

[clang] won’t let me pass &this, but it will let me just pass this. Let me try that.

EDIT: It will possibly be a few hours before I can test on the robot, I apologize for my slowness in advance

I tried your code and it works for me, screen goes green. If I add a delay, it starts red and then goes green.

    foo f = foo();
    f.start_watching(); //start watching the variable
    this_thread::sleep_for(1000);
    f.setvar(); //set the variable to true
2 Likes

How bizarre, I wonder why it’s not working for me.

but I did also not let main exit, that may be an issue for you.
if main exits, foo is out of scope.

1 Like

I’ll try it without allowing main to exit when I get a chance to work with the robot again, thanks.

EDIT:
In the original version of this code, main does not exit, so if that fixes the sample project it won’t fix the original code, unfortunately.

yea, you create an instance of foo on the stack, then main exits and (I forget what happens) but destructor for foo probably destroys it.

1 Like

so create a simple example using std::queue and post that so I can see what you are trying to do.

2 Likes

I’ll get right on that when I have time on my hands, thanks so much.

Okay, here are a modified main.cpp and foo.h that more closely mirror what I’m trying to do. vex::color objects get pushed to the queue, and then the asynchronous thread pops colors from the queue and turns the screen that color. Since there’s now a non-thread-safe collection being managed, I’ve thrown a mutex in there as well.

(In my actual code, I am pushing user-defined Command objects to the queue, which encapsulate commands being sent to different parts of the robot. I have tested the command objects themselves, they are not the source of the issue.)

main.cpp

/*----------------------------------------------------------------------------*/
/*                                                                            */
/*    Module:       main.cpp                                                  */
/*    Author:       charliewade                                               */
/*    Created:      Wed Mar 17 2021                                           */
/*    Description:  V5 project                                                */
/*                                                                            */
/*----------------------------------------------------------------------------*/

// ---- START VEXCODE CONFIGURED DEVICES ----
// ---- END VEXCODE CONFIGURED DEVICES ----

#include "vex.h"

using namespace vex;

int main() {
  // Initializing Robot Configuration. DO NOT REMOVE!
  vexcodeInit();
  
  foo f = foo();
  f.initiate();
  f.submit(color::green);

  while(true) {
    task::sleep(20);
  }
}

foo.h

#ifndef FOO_H
#define FOO_H

#include "robot-config.h"
#include <queue>

class foo {
  private:

    static int callback(void *arg) {
      if (arg == NULL) {return 0;}
      
      foo *instance = static_cast<foo *>(arg);

      instance->watch_queue();

      return 0;
    }

    void watch_queue() {
      while (true) {
        queueLock.lock();
        if (!queue.empty()) {
          Brain.Screen.clearScreen(queue.front());
          queue.pop();
        }
        queueLock.unlock();
      }
    }

  public:
    std::queue<vex::color> queue;
    mutex queueLock;

    void submit(vex::color c) {
      queueLock.lock();
      queue.push(c);
      queueLock.unlock();
    }

    void initiate() {
      task watcher = task(foo::callback, static_cast<void *>(this));
    }
};

#endif

NOTE: I put this together rather hastily, if there’s a dumb error in it let me know and I’ll verify whether that error exists in the original version as well

The above code worked for me, it can be improved, but there’s no fundamental issue.

1 Like

Yes, I just tested it, it worked for me as well. Clearly, something is creating errors in the original version that I haven’t been able to reproduce in this code. I’m going to try to hunt down what that is.

Okay, something very odd is going on here.

I tried instantiating just one foo-type object in my original code, and submitting an object to its queue manually, before any usercontrol code runs. I got, of all things, a Memory Permission Error.

Weirder still, this is the line that is causing the error:

if (!queue.empty()) {queue.pop();}

Correct me if I’m wrong, but that line seems fairly error-proof to me. What the bejezus is going on here?

If it’s truly necessary to look at the original code, and you want to, I can invite you to our team’s github repo to take a look at it. I would just export the project and attach it, but vex forum won’t let me do that yet.

Okay, I fixed the Memory Permission Error; I’m still not sure what was causing it in the first place, but after I removed some calls to Brain.Screen.clearScreen() it started working for some reason.

However, the central problem is still not fixed, although I have discovered that manually creating an object and submitting stuff to its queue does work, but for some reason my functions to do that aren’t working right. I’m going to try to figure that out now.

Ha-HA, I finally got the original code working properly, everything runs nicely now. The error turned out to be this:

The object that needed stuff submitted to its queue was being passed as an argument to a function which handled submitting to the queue. The function’s signature looked a little like this:

void bar(Foo foo);

As it turns out, that actually creates a shallow copy of the object, localized in scope to that function. Since that shallow copy is the one that gets its queue pushed to, the correct one never receives the push, and the item I’m pushing “magically disappears,” since it’s not pushed to the right queue. The signature actually needed to look like this:

void bar(Foo &foo);

That passes a reference to the object, which means that no shallow copy is created, and the proper object receives the push. I’ll tack this onto the list of c++ programs that I have written that have been completely broken for days on end because I was missing one character somewhere in the code.

Thanks so much for your help! I really appreciate your work on this forum.

6 Likes