robotpy / robotpy-rev

RobotPy bindings for REV Robotics' REVLib
Other
8 stars 13 forks source link

Add non-PWM examples #22

Closed Tyler-Duckworth closed 4 years ago

Tyler-Duckworth commented 5 years ago

Somewhat fixes Issue #9 I adapted all of the REV examples with the exception of one, the PWM Arcade Drive since there is not a PWM SparkMax object.

I would note, I did get a lot of errors, but I think that is because the API is incomplete.

virtuald commented 5 years ago

Did you actually test the examples you created? We can't be giving high school students examples that don't work.

Tyler-Duckworth commented 5 years ago

@virtuald Yes, I did test them. The largest issue I had was distinguishing what errors were and were not a product of the API. I found a couple of functions, enums, etc. that I can detail later that weren't implemented in simulations, but worked fine on the RoboRIO. If there is anything you find that show them to be incorrect, please let me know so I can fix it.

auscompgeek commented 5 years ago

Every enum available in the Java API should be available both in simulation and on the roboRIO. From my knowledge, the only methods that have yet to be implemented in simulation are follow and the low-level config API. Let us know if this isn't the case.

Tyler-Duckworth commented 5 years ago

The main reason I didn't port over the Tank Drive example is that I did not know it was there. I was using the C++ examples for reference, but thanks for pointing that out. (The same goes for the Shuffleboard JSON files) As far as the CAN IDs go, usually when I write robot code I configure my CAN IDs to be zero-based like arrays.

I was having some trouble with two main things: rev.IdleMode and rev.CANSparkMax.restoreFactoryDefaults() (Along with rev.CANSparkMax.follow(), but you already mentioned that)

auscompgeek commented 5 years ago

As far as the CAN IDs go, usually when I write robot code I configure my CAN IDs to be zero-based like arrays.

0 isn't a valid CAN ID for the SPARK MAX API...

auscompgeek commented 5 years ago

I was having some trouble with two main things: rev.IdleMode

That one definitely exists in simulation. Does it not for you?

and rev.CANSparkMax.restoreFactoryDefaults()

That one should work fine. There was a bug that would glitch out the simulation UI though that's fixed in master.

Tyler-Duckworth commented 5 years ago

I didn't know that CAN doesn't start at 0 for the Spark Max. Thanks! Also, I figured out my issue with the simulation. I was testing on a laptop that didn't have admin privileges, so I think I had some issues with installing PIP packages. When I tried it on a computer with admin, it all worked.

auscompgeek commented 5 years ago

I was testing on a laptop that didn't have admin privileges, so I think I had some issues with installing PIP packages. When I tried it on a computer with admin, it all worked.

pip works fine without admin privileges; I never run pip as root/admin.

Tyler-Duckworth commented 5 years ago

Got it. I do need to be more cognizant of undoing what I change to properly test these things on my computer.

My apologies for the abundance of errors in this PR. I do need to be much more careful when writing code, especially when it is going to be used by other people.

Tyler-Duckworth commented 5 years ago

Is there anything more you need me to do? I can squash the commits into one, which would make merging easier.

virtuald commented 4 years ago

@Tyler-Duckworth sorry this took so long to approve, I wasn't really doing robotics stuff in the late summer. However, 2020 is upon us, and I just pushed the first 2020 version to pypi/roborio tonight. It would be great if you could try some of these on 2020.

Unfortunately, REV doesn't really support simulation... but I think I would be fine with them as long as they start and don't crash.

auscompgeek commented 4 years ago

For the PID examples, I would also appreciate it if the PID constants were double-checked against the current upstream. I think some of them may have been updated since this was opened.

Tyler-Duckworth commented 4 years ago

@virtuald It's fine! I do want to apologize for the poor first state of my PR. Thanks so much for the feedback about this PR.

I definitely will try out the 2020 version. My team should be handing off a drivetrain with NEOs to me in the next few days, so I will try it then.

auscompgeek commented 4 years ago

Hey @Tyler-Duckworth, we'd love to finally get this merged in. What's the status on this?

Tyler-Duckworth commented 4 years ago

It's been a while since I've updated this with the latest from RobotPy, so there are probably some bugs I need to work out. I'll try to get to them this week!