Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bug Report: mrp not working #150

Open
Zacharyprime opened this issue Apr 5, 2022 · 1 comment · May be fixed by #162
Open

Bug Report: mrp not working #150

Zacharyprime opened this issue Apr 5, 2022 · 1 comment · May be fixed by #162
Labels
Awaiting PR This task is complete and just awaiting a pull request to get reviewed. Clean Up

Comments

@Zacharyprime
Copy link
Member

The discord user "Jammo2000" has reported that mrp is not functional.
He said he recreated the problem using the following code:

#include <kipr/wombat.h>

#define right_motor 1
#define left_motor 3
int main(){
    int i;
	for(i = 0;i<100;i++){
    	mrp(right_motor, 1000, -1000);
    	mrp(left_motor, 1000, 1000);
        bmd(right_motor);
        bmd(left_motor);
        msleep(1000);
        mrp(right_motor, 1000, 1000);
    	mrp(left_motor, 1000, -1000);
        bmd(right_motor);
        bmd(left_motor);
        msleep(1000);
    }
    return 0;
}

Quote from the user:
"So, the move_relative_position (mrp) function in the kipr library is broken. 5-10% of the time it simply fails to move the motor at all."

@Zacharyprime Zacharyprime added the Needs Confirmation This needs to be tested to see if the problem exists. label May 11, 2022
@navzam
Copy link
Member

navzam commented Jul 23, 2022

I was able to repro the issue using the code above. It also repros with mtp, since mrp is just a wrapper around mtp. I think the problem is a race condition in the "motor done" logic - after the "done" flag gets cleared, it sometimes gets immediately set again, resulting in the motor not moving. More details below.


If I understand correctly, the "motor done" flag gets cleared when the motor mode register changes (see wallaby_dma.c#L226-L234):

// handle motors modes clearing done bits
// TODO: cleanup
if (address == REG_RW_MOT_MODES)
{
    if (value & 0b00000011) aTxBuffer[REG_RW_MOT_DONE] &= ~1;
    if (value & 0b00001100) aTxBuffer[REG_RW_MOT_DONE] &= ~2;
    if (value & 0b00110000) aTxBuffer[REG_RW_MOT_DONE] &= ~4;
    if (value & 0b11000000) aTxBuffer[REG_RW_MOT_DONE] &= ~8;
}

The current mtp logic is:

Private::set_motor_mode(motor, Private::Motor::SpeedPosition); // (A)
Private::set_motor_goal_position(motor, goal_pos); // (B)
Private::set_motor_goal_velocity(motor, velocity); // (C)

Say you just called:

mtp(0, 1000, 1000);
bmd(0);

After this finishes, the goal position is 1000, the current position is around 1000, and the "done" flag is true.

Now you call mtp again. Line (A) sets the motor mode, which clears the "done" flag. Before line (B) runs, the STM32 code may run and see that the goal position (the old one) has been reached, so it sets the "done" flag back to true. Now line (B) runs and sets the new goal position, but it's too late. The "done" flag has been set, so the motor won't move towards the new goal position.

I think the easiest solution is reordering the register writes to B, C, A. There can still be different race conditions, but thinking through the possibilities, they shouldn't result in bad behavior.

More complex but possibly better solutions could be:

  • Combine the register writes so they're written together, if possible
  • Move the responsibility of clearing the "done" flag to libwallaby, so mtp can ensure the "done" flag isn't cleared until it's time

@navzam navzam linked a pull request Jul 23, 2022 that will close this issue
@Zacharyprime Zacharyprime added Awaiting PR This task is complete and just awaiting a pull request to get reviewed. and removed Needs Confirmation This needs to be tested to see if the problem exists. labels Sep 16, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Awaiting PR This task is complete and just awaiting a pull request to get reviewed. Clean Up
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants