I started to test the Servo service to be able to make it work again.

I made some testing on version 1723 and the repeated the same tests on the latest version.

The first thing I found is that the rest position is completley ignored when you attach a servo. So independant of how you set the rest position the servo will move to position 0. This is probably the reason that poople have broken robot parts now. I changed the default positoin that a servo will move to att attach to eb 90, but I need to discuss with GroG and other MRL developers about how it should be done. 

I also found that the setController method was renamed to attach, and now has some different logic than it originally had. I think the setContoller method was intended to be used by the GUI's so that the ServoController could be selected in one the Swing gui, and show up as selected in the WebGui, making the Servo sevice the model for both GUI's that would the be viewers. Like in a normal ModelViewerControl concept. But the attach method that it was renamed to,  tries to attach using a pin that may / may not have been assigned.

The setInverted method seems to be broken too. I have not yet digged into the details of that.

 

GroG

7 years 10 months ago

Thank you Thank you Thank you Mats,

I was just about to make a post asking for 'details' regarding why Servo is very bad.  
Its tremedously helpful to have detailed and concise descriptions of the problem.

A little background :

  • The rest thing is horrible - I'll check locally, I don't think its happening on my system, but I'll verify.  I can't check in right now, because there are other changes I need to merge in.. 
  • The setController / attach thing - I started a document, but its unpublished (until now) .. its not "ready" - http://myrobotlab.org/content/attach-pattern  - the crux of it is, I believe the attach method will be very very important, and I think structure & consistancy would be helpful.  Servo is close to following this pattern now .. It would have been better if I did the document first, but you don't always know what you don't know until you do it.

 

I'm gonna test locally the rest thing ... the setController vs attach we probablly will need to discuss further - but it would be nice if you read my 'incomplete' documentation regarding attach and give me your feedback.

Thanks again Mats for starting this thread...

calamity

7 years 10 months ago

the new attach method can take argument for the initial position of the servo and velocity

it look like this

servo.attach(ServoController, [pin], [position],[velocity])

I think the basic servo.attach(ServoContoller) was made so it's possible to use the attach in bidirectionnal way (servo.attach(arduino) or arduino.attach(servo)). That's a great feature, but nothing in that code check if pin or position have been previously set and my potentially use null as pin and 0 as targetPos.

the attach(servoController, pin) version should definitively use rest as default position.

The setInverted is definitively broken, I have notice that some time ago. I think the problem is that it swap the input data instead of the output data to follow the logic Mapper had.

Locally I have :

servo.attach(controller, pin) & controller.attach(servo, pin) working - 

additional parameters are possible .. position & velocity

if position is not specified, it is taken from rest .. rest's default value is 90

this may be a case of merge or a half commit .. my local build doesn't look like it has this problem, but I need to do further testing... 

OK

When do you think that your version can be pushed. I want to continue testing, but prefer to do in the latest version.

/Mats

I had compiling errors because of the refactoring of heartbeat/publishBoardStatus/publishBoardInfo changes ..

I have it compiling now and did some very basic servo tests - so I'll push now

GroG

7 years 10 months ago

I hope we all can agree that attaching a pin is a simple function of a library ..
Arduino's library allows us to detach and re-attach to a different pin, the controller does not need to be removed to do this ...  

Attaching a controller or another service is a different concept.  
We should keep the granularity of being do the different methods .. e.g. change pin without controller without changing pin & change pin without having to change controller

The default action of attaching a controller will also attach the pin specified.

The pin is the only data which cannot be defaulted.

Attaching a pin and detaching a pin .. is just that .. calling attach(pin) & detach on the Arduino library..
"new'ing up a new Arduio Servo" is not needed or wanted... and this would happen if we use the attach/detach(controller)

Some things which are not defined is .. 

  • When you detach a controller - move the servo (or don't move the servo) then re-attach the controller - what is the expected results ?  - there are lots of other variables with this - e.g. when the ui does it..