Skip to content
This repository has been archived by the owner on Jul 3, 2019. It is now read-only.

Unify gamepad and sensor control into two seperate super classes #61

Open
climathosphere opened this issue Feb 1, 2017 · 3 comments
Open

Comments

@climathosphere
Copy link
Contributor

After looking at the drive and arm processes, it came to my mind that the ArmProcess and the DriveProcess can extend from a super class because they use similar functions and subscription keys. For good object-oriented programming practice, it makes alot more sense to create a super class for a group of classes that share functionality.

For example, since the DriveProcess and the ArmProcess share functionality they can both inherit the similar functionality from a super class (something like a "JoystickProcess" class that extends the RoverProcess class). The JoystickProcess class can then handle the similar functionality universally of all of its subclasses and then the subclasses can deal with all the methods that can only be distinctly invoked for their own specified processes.

@ottopasuuna
Copy link
Member

This is definately a possbility, the DrillProcess would benefit from this as well. The only reason I'm a little bit hesitant, is that all three modules end up using the joysticks in slightly different ways, for example the arm might use joysticks to control motor position, the drive process will select between rpm and current control, and braking, the arm might use two axes (the triggers) instead of one to turn a positive/negative direction, etc. The way I'm envisioning the code right now, we would end up overriding every method of the super class anyway, negating the benefits of the common parent class.

That being said, you might have better ideas of how to structure this change to avoid the problem I listed above.

My proposal is to keep things the way they are for the month of Feburary to make sure everything works for the critical design review. After that, we can refactor the Drive, Arm and Drill processes to take advantage of a common super class.

@ottopasuuna ottopasuuna modified the milestone: March 2017 Mar 3, 2017
@climathosphere climathosphere self-assigned this Mar 18, 2017
@ottopasuuna ottopasuuna modified the milestones: March 2017, April 2017 Apr 1, 2017
@ottopasuuna ottopasuuna modified the milestones: May 2017, April 2017 Apr 30, 2017
@ottopasuuna ottopasuuna changed the title Create and use a super class, an abstract class, or an interface for processes that have almost identical funtionality. Unify gamepad control into a super class May 22, 2017
@climathosphere climathosphere changed the title Unify gamepad control into a super class Unify gamepad and sensor control into a super class May 27, 2017
@climathosphere climathosphere changed the title Unify gamepad and sensor control into a super class Unify gamepad and sensor control into two seperate super classes May 27, 2017
@ottopasuuna ottopasuuna modified the milestones: June 2017, May 2017 Jun 2, 2017
@climathosphere
Copy link
Contributor Author

climathosphere commented Jun 15, 2017

I currently have three plans for which I can approach this issue.

These two plans deal with implementing the classes in the current rover system:

  1. The first one is to leave the super classes currently as they are as Interfaces and override them when they need to be used.
  2. The second one is to see if the super classes can be made as abstract classes where they can contain some functionality. However, if a process needs specific traits tailored for its use, then it can add functionality by extending the abstract class and adding the necessary features that are required.

The third plan (and probably my best one) deals with implementing the classes in the new rover system:

  1. The third one is to create a versatile library of classes (such as a class tree and branches out like a real tree :D). This plan includes:
    a. Using a class for each communicable device. This means making a class for each motion-by-command and sensor device instead of hard-coding message and device constants all over the place.
    b. Separating the hierarchy of the classes so that parts can be distinguished based on their functionality/features and can be individually tested without testing other parts.
    c. Have a class for each language or external operating system that is being used. The reasoning is reasoning is the same as in bullet point a: less hard-coding, more reusability. (This is disputed with the 0MQ module).

This approach should meet around 5-6 recommendations that were specified in SoftwareOverview2017.pdf published on the software channel.

@ottopasuuna
Copy link
Member

Option 2 is what I would like to see for now. I was originaly thinking something like this:

def register_filter(self, message_in, scale_func, messages_out, vesc_msg):
    def callback(data):
        x = scale_func(data)
        for message in messages_out:
            self.publish(message, vesc_msg(*x))
    self.message_lut[message_in] = callback

def message_trigger(self, message):
    if message.key in self.message_lut:
        message_lut[message.key](message.data)

This will blow up if your scaling function doesn't take in or return the right data types, but that can be checked.
Then, a rover process can define scale_func, and call register_filter in its setup method with the appropriate parameters, and when message_in is triggered, the appropriate vesc_msg will be published for each key in messages_out.
^ This is all just a suggestion though, there may be other ways to implement the same functionality.

We'll discus next years plans in detail sometime after the competition in a month, I especially like your point b about being able to test functional units individually. As long as the rover software doesn't end up like this we'll be good :)

@ottopasuuna ottopasuuna removed this from the June 2017 milestone Jul 22, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

2 participants