Issues with Rev SparkMaxRelativeEncoder

[Dylan_Watson]

My apologies for making two of these threads, the first one was an accident.

For a number of years now, my team has written wrappers around robot component classes (SparkMAX, VictorSPX, etc.) in order to use some of our own logic (all written in C++). When working with the updated REVLib and CANSparkMax, we wanted an interface in order to easily get the position of the encoder. For reference, we only use SparkMAX controllers for NEO motors, so it is always a kHallSensor encoder.

In order to get the encoder, it seems that we have to call the GetEncoder() method, which returns a SparkMaxRelativeEncoder object (not pointer). Our first attempt was to call this method whenever we needed to retrieve an encoder position, however we quickly realized that this throws the error GetEncoder() has already been called for SPARK MAX.

After this, we tried holding the SparkMaxRelativeEncoder as a member variable of our wrapper class. However, SparkMaxRelativeEncoder doesn’t have a default constructor, which is necessary to hold an object as a member variable in a class as far as I know.

Does anyone have any tips on this? I hope I am just missing something simple within REVLib and overthinking this. Thanks!

[GodCed]

(I’ll assume C++ because you mention pointers)

Does this example work for you?

I don’t see your full code but you should be able to call GetEncoder() to initialize your member, thus avoiding repeated calls and bypassing the default constructor problem.

[Dylan_Watson]

We map all of our components, and the CAN ID’s associated with them, to an XML file that is read on code startup. Because of this, the CANSparkMax itself is not initialized right in the wrapper class definition, but instead in the constructor of our wrapper class (I hope that makes sense). This leads us to having rev::SparkMaxRelativeEncoder enc; in our wrapper class’ member variables, and enc = motor.getEncoder() in the wrapper class’ constructor, which presents the default constructor issue.

[GodCed]

Gotcha. Then you’ll have to store a pointer to your encoder, but make your own pointer! That should work, as the copy constructor for the encoder class is defined but it’s untested.

// wrapper.hpp
class Wrapper
{
  // Encoder smart pointer
  std::unique_ptr<rev::SparkMaxRelativeEncoder> m_encoder;

  // Drive
  rev::CanSparkMax m_drive;
};

// wrapper.cpp
Wrapper::Wrapper()
{
  // Construct your drive
  m_drive = ...;

  // Get the drive's encoder and store a copy into your pointer
  m_encoder = std::make_unique<rev::SparkMaxRelativeEncoder>(m_drive.GetEncoder());
}

double Wrapper::GetEncoder()
{
  return m_encoder->GetWhatever();
}

[Dylan_Watson]

Oh! I had thought about this, but tried it without unique_ptr, I just did it with rev::SparkMaxRelativeEncoder * enc; and enc=&(Max->GetEncoder());

I will give this a shot at the robotics shop later today. Thank you!!

[GodCed]

No problem. FYI, what you did with old style pointers should have worked like this:

rev::SparkMaxRelativeEncoder* enc = new SparkMaxRelativeEncoder(Max->GetEncoder());

The trick is that you must create your own copy of the encoder object, else your pointer will point to garbage when the original object (returned by GetEncoder()) goes out of scope at the end of the constructor.

But really old style should not be used anymore because they put the burden of memory management on you and a good programmer is a lazy one.