Absolutely agree! I have seen this happen many, many times. Also why would you want to train students to do things in a non-standard way.
I strongly disagree with this statement. The purpose of the setInvertedMotor() method is to handle motors which should be logically reversed because they are on the other side of the robot (ie. in the case of the drive train). This method only needs to be called once for each motor controller object (eg. Talon or Victor or Jaguar) when it is created. There would be no need to call it multiple times or to keep track of anything related to it. Using this method is the correct way to implement this. It is part of the programmers task to understand how the robot was built and what accommodations need to be made to properly interface to the parts.
Inheritance is the best way to do it. It’s much clearer to say “ReversedMotor” than making a motor and reversing it later. What happens when one of your programmers wants normal motor behaviour (not reversed) later down the line? They call your inverse method. And if they neglect to switch back, you’re in for some deadly consequences.
Another problem is the time between creating the object and reversing it. When is the correct time to initialize the reversed state? How can you guarantee that methods aren’t called before that? (By accident or on purpose - think static initialization)
Always prefer inheritance and polymorphism to state. It’ll help you in the long run. Settings for things should rarely be flags if you have better tools to use.
-all motors are red-red and black-black in code (or + to red if its not colored)
-all analog sensors are wired the same between both robots (usually they are magnetic encoders or gyros which have polarity anyway but a potentiometer has no polarity)
-i haven’t standardized on the digital sensors and digital actuators but I should probably set one as well.
-the ‘application’ code always reads the sensors using the forward/up/left standard and in engineering units and writes the motors using forward/up/left as well (with ±1 range)
-the boundary layer code handles negating and scaling (gain/offset or interpolation table) of sensors and actuators usually all in one file (well two, inputs and outputs)
-I have gravitated over the years to pulling the boundary layer code out of the application code for scaling, inversion, and selective disabling/override testing
I find it hard to concur with this opinion. If you have more than five minutes, get your programmer over, and do it right. As it has been brought up other times in this thread, if a motor breaks and needs to be replaced, there’s a good chance that if you had the wires reversed, they won’t be reversed when the motor is replaced. This can be catastrophic, especially if the motor in question is part of a 2-motor gearbox. I can see switching the wires if you have one minute until your next match, but if you have time, switch it in code.
In exactly the same way that your derived ReversedMotor class would always use the motor in that reversed “state”, so too the SetMotorInverted() method can/should be called only once during it’s instantiation (ie. in the constructor for the object it is part of) and therefore it would remain in that state during it’s entire existence. It should not matter to the programmer after that initialization if the motor was inverted or not. That is the purpose of that method and that internal state. If that is not true (ie. if you might sometimes want to use it non-inverted and sometimes to use it inverted) then you should not be using that method you should design algorithms and coordinate systems to properly reflect the usage of that motor controller. If properly designed, the positive direction of the motor is consistent through out its usage.
My perspective would be that there is no need to derive additional classes when the functionality is already properly provided by the existing class.
The only time to be lazy and invert the wiring is when the code you’re dealing with is a ‘black box’ that is prohibitively expensive to change. Even then, there should be some sort of symbol on the wires that the wires are reversed intentionally.
This shouldn’t be a factor in FRC because we’re at least given enough time to do things correctly from the start.
For example, a legacy version of APM I used for my Quadrotor last year put all 4 propellers going the same way (i.e., no flight). The ‘right’ way to fix it was to dive into the firmware, find the bug and then spend a large quantity of hours learning something I wouldn’t spend a second on after an official bug fix. Instead of wasting 20 hours, I reversed two motors’ wiring so they spun to match the diagram and then put red/black zipties around the individual wires to signify which wire (even which side of the connection) was reversed.
However, the thing that Joel was saying earlier in this thread is that by using inheritance, you can use polymorphism to your advantage and then have the code only execute the reversed state at run time. Also, like Joel mentioned, the cleanest way to do it is do have the inverted state handled in the constructor of the reversedMotor class. This reduces the amount of code you have, which makes the code cleaner and much, much easier to diagnose if there is something wrong.
I see your perspective and understand how inheritance and polymorphism work, it is just a difference of opinion as to which is cleanest. In my experience it is cleaner, especially since it is a single function call, to execute the setMotorInverted explicitly rather than hide it inside a derived class which does nothing more than that. In particular, if someone outside of your team were to look at your code (perhaps an FTA trying to help you look at a problem), they would need to look at the ReversedMotor class to determine what else might be encapsulated inside it and how that effected your code.
But the same applies to using setMotorInverted. You need to be able to find where that function is called, and when it happens. The motor is no long defined as inverted, but rather is set as inverted somewhere along the line. Sure, in practicality it probably won’t cause problems. But it’s much clearer to OO programmers to see
Talon t = new ReversedSpeedController(new Talon(1))
I like to have one class per mechanism or subsystem, and these classes then have private instances of WPIlib primitives (Victors, Talons, encoders, etc.)
The rationale is as follows. The mechanism-level class knows “how” to do something. No other part of the code should know or care about the details (are we using Victors or Talons, what sensors, what PWM ports, what PID gains, are motors inverted…) If I need to look at how something works, there is one place and one place only that I ever need to look for any particular function of the robot. The benefits of this approach are numerable.
Within this framework, the scope is narrow enough that whether I use a ReversedSpeedController, call setInverted, or just multiply all the inputs by -1 it really doesn’t matter. The point is that ALL the code that ever touches that speed controller is co-located in one cohesive package.
Agreed. This is how I would do it and since child classes in Java only have single inheritance (inherent only the immediate parent’s methods), this would be fine as long as you don’t make a hierarchy with more than 4 levels (although I don’t really see a situation in which you would).
I do not see why motor wires are even color coded. They should be solid white, because a motor controller will anyways switch the polarity on demand.
They should add a jumper to the motor controllers for whether the motor is reversed or not. The sad thing is that you cannot just flip the motor over