pdx-robotics / Arduino_KHR-1

Control KHR-1 with Arduino
4 stars 0 forks source link

Implement semaphore #25

Closed halyngoc closed 6 years ago

halyngoc commented 6 years ago

This is a major rewrite of the library.

Includes changes from PRs #21 and #23. Closes #14.

Major changes include:

To-do:

geoolson commented 6 years ago

speed_test.ino could be archived or removed. The VarSpeedServo library isn't being used for KHR1

halyngoc commented 6 years ago

I've noticed that master is too outdated compared to this branch which makes working on other issues more difficult. So I'll look into getting this merged as soon as possible. Some of these tasks in the to do list above are not essential and could be turned into separate issues after merging.

williamVbrines commented 6 years ago

To avoid conflicts what can I help out with that needs to get done so we can merge with origin.

halyngoc commented 6 years ago
  • [ ] Change the order of Pose's members. Currently the order is left arm, left leg, right arm, right leg. I want to change it to left arm, right arm, left leg, right leg to make it more readable

Imo, we'll do some testing (#14), then I push the code for this task, then more testing and we're ready to merge. The rest can be done later.

Right now though we can only wait for KHR_1 to get fixed up. You can help with #26 if you're able.

geoolson commented 6 years ago

On line 125 in the header, pwm is initialized. Declare pwm in your header but initialize it in the constructor in the cpp. The init function is unnecessary as well. Move the body of your init function into the constructor of your cpp.

halyngoc commented 6 years ago

The init function is unnecessary as well. Move the body of your init function into the constructor of your cpp.

As i remember, there used to be attach() calls in the constructor which created #3. Had to move them out in a separate attach() function, which then got replaced by this init(). Idk if the problem is still there but I don't wanna risk it.

halyngoc commented 6 years ago

On line 125 in the header, pwm is initialized. Declare pwm in your header but initialize it in the constructor in the cpp.

As for this, idk how that escaped me 😃

williamVbrines commented 6 years ago

I would say that semaphore has been successfully implemented, tests have been made on the semaphore program. If there are bugs or enhancements we feel that need to be added or fixed we can make a new branch. As to the conflicting files

I feel like this branch has done what it has meant to and should be merged to the master.

halyngoc commented 6 years ago

These conflicts are too complex to resolve in the web editor

We'll have to do a merge from master and try to resolve the conflicts that way

halyngoc commented 6 years ago

Conflicts resolved. Although the 2 files modified by 1b2043f still have problems that can be fixed in a separate issue.

halyngoc commented 6 years ago

I don't think attach() exists anymore so I changed it to init() but I'll open an issue on these files later.