robotpy / robotpy-ctre

RobotPy wrappers for CTRE Phoenix 5 library
Other
13 stars 21 forks source link

Add current limiting functions #8

Closed juliaschatz closed 7 years ago

juliaschatz commented 7 years ago

setCurrentLimit function, giving the current limit in Amperes.

enableCurrentLimit function, enabling/disabling the current limiting function.

9.3 in the software reference manual.

cjlawson02 commented 7 years ago

I second this, as this is a very important feature to protect our hardware.

virtuald commented 7 years ago

I'm hoping CTRE will rearchitect their plugins such that we can support all APIs in the future. Right now it's too easy to miss things.

juliaschatz commented 7 years ago

Here's my go at it: https://github.com/nickschatz/robotpy-ctre
The handles are all there, it just needs to be attached in Python. I don't have ready access to a rio/talon anytime soon, so it would be great if you can test this for me.

Compiled wheel

cjlawson02 commented 7 years ago

Got it, thanks! Will be working on it today

On Sep 2, 2017 at 8:03 AM, <Nick (mailto:notifications@github.com)> wrote:

Here's my go at it: https://github.com/nickschatz/robotpy-ctre The handles are all there, it just needs to be attached in Python. I don't have ready access to a rio/talon anytime soon, so it would be great if you can test this for me.

Compiled wheel (https://www.dropbox.com/s/tgxhqbms9s1wneh/robotpy_ctre-2017.0.3.post0.dev2-cp36-cp36m-linux_armv7l.whl?dl=1)

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub (https://github.com/robotpy/robotpy-ctre/issues/8#issuecomment-326749521), or mute the thread (https://github.com/notifications/unsubscribe-auth/AG7rFfIZR4CfHGnBHeJ4iaE1Im-s5Pz9ks5seW49gaJpZM4O60vu).

cjlawson02 commented 7 years ago

I can build successfully on my computer, but I'm having trouble getting it working on the rio. @virtuald How do you add support to the roborio ctre?

cjlawson02 commented 7 years ago

@nickschatz where did you find the hal_data param for the current limit? Also, where do I use the whl file? Is that the file that defines the current stuff for the rio?

juliaschatz commented 7 years ago

@Chris2fourlaw I can't link because I'm on mobile, but it's all in the cantalon_roborio.cpp file. The wheel file is the compiled code from my repo. You can use pip to install that on your roborio and get my current limiting code. You can't just toss this code in the rio, it needs to be compiled. I can share my setup for that if you want, because it took me all of forever to figure it out.

cjlawson02 commented 7 years ago

@nickschatz Oh yeah, I see it now. I'll have to try again later when I have access to the rio. I'd also love to see your setup.

juliaschatz commented 7 years ago

https://gist.github.com/nickschatz/0c9872be5195e8306383d940ff5094b2

Here's what I needed to do to compile robotpy-ctre. Like I said, most things are implemented in C++ and just need the python wrapper functions and sim support added.

Edit: I just looked around some more and while current limiting was pretty close, it's not trivial to implement the other issues you posted.

virtuald commented 7 years ago

I've sent an inquiry to CTRE about the status of their updated API, which should make it significantly easier to add support for the entire available API. I'll wait until I hear from them about that -- though if @nickschatz wants to PR his changes, I'll merge it.

juliaschatz commented 7 years ago

@Chris2fourlaw Have you tested my changes? I'll probably get to it tomorrow if you haven't.

cjlawson02 commented 7 years ago

Oh sorry! Yeah I did and I believe it’s working. What’s the best way to confirm?

On Sep 11, 2017 at 2:49 PM, <Nick (mailto:notifications@github.com)> wrote:

@Chris2fourlaw (https://github.com/chris2fourlaw) Have you tested my changes? I'll probably get to it tomorrow if you haven't.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub (https://github.com/robotpy/robotpy-ctre/issues/8#issuecomment-328668997), or mute the thread (https://github.com/notifications/unsubscribe-auth/AG7rFRQrg2OqKz7fPOTdOWGe4mKT9xVoks5sharogaJpZM4O60vu).

juliaschatz commented 7 years ago

For starters, not crashing is a good sign. I would probably put a low limit on it and then just stall out a CIM at low power while monitoring current.

cjlawson02 commented 7 years ago

Alright, well it didn’t crash. I can test the current on Thursday, but if you can do it earlier that’s better.

On Sep 11, 2017 at 9:10 PM, <Nick (mailto:notifications@github.com)> wrote:

For starters, not crashing is a good sign. I would probably put a low limit on it and then just stall out a CIM at low power while monitoring current.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub (https://github.com/robotpy/robotpy-ctre/issues/8#issuecomment-328730626), or mute the thread (https://github.com/notifications/unsubscribe-auth/AG7rFfxHBsi2us5ZKUECo6EckPvVNxO7ks5shgQlgaJpZM4O60vu).