Button can only be clicked once?

This code doesn’t work to check button clicks.

int button::CheckCollisions()   {
  while(true) {
    waitUntil(Brain.Screen.pressing());
    point p = {Brain.Screen.xPosition(),Brain.Screen.yPosition()};
    if (checkCollsion(p, this->s) && isDisabled != true) {
        if(hasSender)
        {
          this -> senderFunctionPointer(senderArgs);

        }
        else
        {
          this -> functionpointer();
        }
      }
    }
  return EXIT_SUCCESS;
}

This code does work to check button clicks but only once then the button doesn’t work (As expected).

int button::CheckCollisions()   {
  while(true) {
    waitUntil(Brain.Screen.pressing());
    point p = {Brain.Screen.xPosition(),Brain.Screen.yPosition()};
    if (checkCollsion(p, this->s) && isDisabled != true) {
        if(hasSender)
        {
          this -> senderFunctionPointer(senderArgs);
          return EXIT_SUCCESS;
        }
          this -> functionpointer();
          return EXIT_SUCCESS;
      }
    }
  return EXIT_SUCCESS;
}

(These are being ran in a task).

If anyone needs any additional info please reply.

This is a classic case of the code is doing exactly what you’re telling it to do. The first one isn’t working because your ‘while’ loop has no method to exit. Your second ‘while’ loop exits because you have a return path.

There are multiple ways to solve this, but chief among them is don’t use ‘while(true)’ statements inside functions that you want to exit out at some point. Consider instead looping on a variable that can be changed once you’ve met the condition. Consider instead ‘while(screenPressed == 0)’, and setting ‘screenPressed’ to 1 when you detect what you’re looking for. This then allows the loop to exit naturally and then execute your return call.

Also, I would highly recommend adding a wait/sleep call in the 20-50ms range at the end of the loop to give the reat of the system time to do things.

7 Likes

How would I know if the button is being pressed if I didn’t always check it?
Also I don’t really care if the function exits out at any point just that it detects the button click and calls the function.

not enough to go on there, put together a small example project that demonstrates what isn’t working and I will have a look. (ie. create and send me a simple VEXcode project I can just open and run)

8 Likes

I missed the part where you said this was a task, my apologies. Too much robotics today followed by much too much work. Still, it seems to me like something related to how the while loop is executing, if the only difference is adding in returns.

Agree with jpearman, need more context around the rest of what’s happening in the program to be able to diagnose.

But also, still, add a sleep/wait inside that loop so your other tasks can have some CPU time. That advice doesn’t change regardless of what the rest of the program looks like.

4 Likes

code.zip (152.6 KB)
txt file inside to explain stuff

ok, I’ll run it tomorrow. It’s a bit more complicated than I really wanted as a simple demo, but I’ll take a look, first reaction is perhaps something to do with all that std::function code causing issues.

6 Likes

I ruled out buttonManager. It could still possibly be the problem I doubt it though.
Also people helping me debug some code wrote the launch_task in button manager. Not sure how the syntax works.

ok, the code has many issues, and I have to work today so have no time to try and fix all of them for you.

fundamental issue was that after the powerSettings button is pressed, you setup the new screen and then exit the CheckCollisions function, that button will not be checked again. I added some debug code to help determine what was going on.

int button::CheckCollisions()   {
  int count = 0;
  
  printf("CheckCollisions entry %d %08X\n", (int)this_thread::get_id(), this );

  while(true) {
    // wait for no press
    while( Brain.Screen.pressing() ) {
      this_thread::sleep_for(20);
    }
    
    // wait for press
    waitUntil( Brain.Screen.pressing() );
    
    printf("press %d %d %d %d %s\n", count++, this->s.x, this->s.y, isDisabled, this->text1 );

    point p = {Brain.Screen.xPosition(),Brain.Screen.yPosition()};
    
    if (checkCollsion(p, this->s) && isDisabled != true) {
        printf("%s pressed\n", this->text1);  

        if(hasSender)
        {
          printf("exit here 1\n");
          this -> senderFunctionPointer(senderArgs);
          return EXIT_SUCCESS;
        }
          printf("exit here 2\n");  
          this -> functionpointer();
          return EXIT_SUCCESS;
      }
  }

  printf("exit here 3\n");  
  return EXIT_SUCCESS;
}

The original code also was constantly running the loop when a button was pressed, so I added code to make sure button (well, screen touch) was released.

but there are other more fundamental issues that I’m not sure how to diagnose just yet related to task/stack use and the way you are creating them on the heap. Not sure how the std::function you create using new is destroyed and how that will conflict with the internal vexos scheduler.

Every button/UI class I’ve created uses only a single task for button monitoring, I really don’t think you should go down the path of having a task per button that monitors for screen presses, my gut says this will just cause more and more problems for you.

The UI class I created for the field control application has the following class hierarchy

Top level UI class that is subclass of vex::brain::lcd (so it can use those member functions) that has a list of “pages” with one page designated as the current displayed page. There are a bunch of functions to manipulate pages and static member functions for dealing with screen press and release.

A “page” has a number of on screen buttons, a button being a class with an on screen area as well as many other properties dealing with how it’s drawn etc.

The screen press static member function looks like this and is called every time the screen is pressed (ie. it was registered using pressed function on the lcd class )

void
ui::_screenPress( void *arg ) {
    if( arg == NULL)
      return;
  
    ui *instance = static_cast<ui *>(arg);

    if(!instance->_enabled)
      return;

    int xpos = instance->xPosition();
    int ypos = instance->yPosition();
    if( instance->_fullscreen )
      ypos += 32;
  
    for ( auto &b: instance->_current_page->_buttons) {
      if( b.inside( xpos, ypos ) && b.enabled ) {
        if( b.toggle == true )
          b.state = (b.state == true) ? false : true;
        else
          b.state = true;
        
        b.pressed = true;
        
        if( b._redraw ) {
          b._redraw( b );
        }
        if( b._action ) {
          b._action( b );
        }
        return;
      }
    }
}

without getting into all the details, it iterates through all buttons on the current page to see if the press was inside the area that defines the button on screen. If it is, actions and redrawing are performed. There’s a similar function for screen release. If a page is not active then no buttons for that page are tested.

So ultimately it’s up to you how you want to implement any of this, but my advise is use the Brain.Screen.pressed and Brain.Screen.released events rather than creating a new task per button.

5 Likes

a little more info. Things like this will cause issues.

  
  button buttons[1] = {back};
  
  bm.addButtons(buttons, sizeof(buttons) / sizeof(button));
}

you make a copy of the button into an array, but the array is on the stack, and that instance is destroyed when it goes out of scope, probably not a good idea.

5 Likes