Critique of joystick driver after adding a new adaptor.

[Carl Ranson]

So I just spent quite a while configuring a new joystick in gobot.io.  Its a magicsee r1 (Image below)

It wasn't terribly easy to achieve so I wanted to offer the following ideas for improving the code.

1. joystick.NewDriver method, not clear what the config parameter is supposed to be. 
It wasn't until I dug into the code that it was apparent that the config parameter could be either

one of the special strings "dualshock3", "dualshock4", "tflightHotasX", "xbox360", "xbox360RockBandDrums"

OR

a path to the config file for the joystick. In my case i used a fully qualified path ""/home/pi/go/src/gobot.io/x/gobot/platforms/joystick/configs/magicseer1.json"

If there are special keys, they should be factored out as constants. Also, the documentation should state that the parameter has two possible forms. 

In addition, it isn't clear where the path is evaluated from if a relative path is used. 

2. handle event unknown button silent fail. 

The code in handleEvent matches the button value against the config file. If there is a mismatch and the value isn't found, there's a silent fail. 

This case should show up somewhere as an error to catch problems such as misspellings in the config file. Same goes for axis and hat mappings

3. loadFile silent failures.

There is a path here that should be caught and shown as errors. the call to json.Unmarshal returns an error but it is discarded. Having this returned as an error would catch cases where the config file was missing a comma or something (surprisingly easy to do)

I hope I've explained the problems clearly there and suggested worthwhile improvements.

CR

[Carl Ranson]

and when I say adaptor in the title I mean config file. its been a long day. 
CR

[Carl Ranson]

oh, i must apologise.
I was wrong about point 2 in my post. the handleEvent method does, in fact, emit an error if it can't match the button to the driver. 

but, I also noticed another fault, the Start method ignores any error thrown from the loadFile method making it harder to tell why a new config isn't working. 

CR

[Ron Evans]

Hi, Carl

Thanks for all the feedback. Seems like some additional docs are needed to explain a few things about how the driver currently works.

The short version is, to avoid the need for an additional external JSON file, the most recent Gobot lets you load one of the pre-installed drivers aka dualshock4. We did not remove the JSON feature, ideally in order to better support new devices being added, cleanly with only limited success.

Your thought about some const values seems like a very good idea. Same with some error checking when loading a JSON file.

If you feel so inclined, perhaps you could create a Github issue so this information can be tracked?

Also, it would be great to receive a PR with your new joystick, if you find the time.

Thanks again for the feedback, and using Gobot.

[Carl Ranson]

Hey Ron, 

Don't think I'm officially on Github but I'll look into it on the weekend. 

As for sharing my config, I'd be more than happy to. This little thumb joystick is a really cool gadget.

I also have a driver for an adafruit motor driver board. I don't have much time to curate that through the submission process, but maybe we can work something out where I pass it onto you and you take it home. (it's pretty simple) 

Great to see someone from the project engaging with the user, btw. I really think go could be a fantastic language for bot development. 

regards

CR.

[Carl Ranson]

cheers Ron, Converted this post to a couple of change requests. Will look into submitting my config file at a later date. 

CR