Hi,
 
I'd like to get some input on this interface and a little feedback on how to make it stronger or more clear.
 
 
Kwatters said the pinEnable(String pin) method in Arduino is confusing. 
I added this method because previously he said he wanted to pinEnable(D0) .. I guess I thought he meant pinEnable("D0") .. which currently works, but I'm not a big fan ..
 
I'd be fine removing pinEnable(String) .. but I'd like us to agree what would be the alternative to use in Python.
 
Some find the 'real' address disagreable, because its clearly documented what the value is :  e.g. pin 14 is A0 on an Uno.  
 
Soooo...
Plan is to get rid of pinEnable(String) in Arduino and all pin as String methods,
And make a bunch of constants ?

Constant ints would be ok ..  public static final int D0 = 0; ..  public static final int A0 = 14;
but in Python it would look like this - which is rather verbos, but ok by me...
pinEnable(Arduino.A0)
 
Let me know your thoughts and opinions...
Cheers...

Mats

7 years 7 months ago

const A0 = ?

The value is depending on the board. Adruino Uno and Arduino Mega has different assignments of the analog input ( + the Mega has more analog inputs ).

So I don't think it could be a constant.

 

The way I think about unit tests, is that when you create a service, you test it so that it works.

Then you create unit tests to make sure that future changes dont' break backward compability.

I don't know how many people use the String version of enablePin, but removing that method would break backward compability. 

I have seen examples of enablePin in both the integer and String versions.

The virtualArduino example uses the String version, so that example would be a no worky example. And that example was created just a few weeks ago. 

http://www.myrobotlab.org/service/virtualarduino

 

Sure, and all the Python that is in pyrobotlab could certainly be updated... but of course, there is an unkown amount of use out there (for this one I expect not that much, as I know enablePin(String) is relatively new compared to the lifetime of Arduino service)

I posted this blog hoping I would get some input.  With working integration tests & virtual arduinos we are headed to a more capable and more robust and stable releases.  
But, instances will continue to arise where a method should be removed because of a mistake, or to prevent further confusion or to simplyfy or reduce the complexity of the interfaces.  Its part of growing...

There is a cost in fixing things there is also a cost of not fixing things... and "fixing" can often be very subjective.

Now the hardpart ..

  • does it need fixing
  • how should it be fixed
  • what is the total risk
  • how can the risk be mitigated
  • what is the cost of leaving it
  • and bototm line - "Does the total risk warrant the benefits of the change?"
  1. I put this trying to figure out a way to make it better, because kwatters said it was confusing to have the callback of pindata with a pin address of 15, I am responsible for the current issue.
  2. I see it as costly, because I think the whole MRL messaging system might not support overloaded methods, especially if the are primitive parameters.  This could be another thing to "fix" - but that will take more time. enablePin(int) & enablePin(String) might randomly error if called remotely...
  3. Its relatively new, so I think outside of pyrobotlab and myrobotlab repos its not used .. this is only a guess - who can say ?
  4. I know we talked about leaving a method as @Deprecated for a whole release then next release removing it .. but how do Python users see @Deprecated - perhaps studying the logs .. hmm  if there is a "warn("enablePin(String) is deprecated") that would go all the way to the gui .. hmmm
  5. This method has not been formally released - its only on develop branch
    https://github.com/MyRobotLab/myrobotlab/blob/master/src/org/myrobotlab…    Do I need to put it in a release just to take it out ?

And finally how to make it less confusing ?

is enablePin(Arduino.D34) the best way - what is the value of Arduino.D34 on an uno ? (I would guess just remap - like digitalWrite(34) does on an Arduino ..

Thanks for the input Mats..

Not sure about the "best way", but something that can be used both in enablePin and when the callback is executed. Based on your earlier suggestions of not using primitives, I guess that enablePin(Integer pin) should be the way to go, and that enablePin(String pin) should go away.

I think enablePin(Arduino.D34) is a good suggestion, but for an Arduino Uno it should throw an error, since D34 doesn't exist on the Uno.

Point 2 is a very good reason to fix it. It's a point that I didn't think of before, but I understand the problem now.

I think point 5, it has not been in a release may justify the removal of enablePin(String pin), and that the virtualArduino example could be updated. 

I have always like to use constants identifier instead of int or string to pass parameters, so I won't complain using enablePin(Arduino.A1)

You can easily keep the 'Constant' behavior and the id of the pin in function of the boardType using a enum type that have the the method getId(boardType)

  • The advantages are
  • keep the constant behavior, Pin.A0 is always the same 'value', but can return different id depending of the boardtype
  • Arduino do not have to care about the pin mapping as it can be done by the enum

Disavantages

  • Python usage will look more like enablePin(Arduino.Pin.A01)
  • I never see you use the enum type, so you probably don't like it, maybe for a good reason

I'm on my mobile now so this post will be a bit short.

Generally the arduino supports 2 types of pins: Analog and digital. Some digital pins support pwm, but they are still addressed as digital pins.

In the arduino code, they are typically referred to at A0 for analog pin 0 or D0 for digital pin 0.

So, I think the interface in mrl Arduino should have a similar nomenclature... When addressing a pin we should provide both type and pin number.

Currently, as just an integer it's quite confusing because the pin numbers will differ between an uno​ and a mega.

Ultimately, we need to support some pass through sort of features on pins such as pin mode, analog write (which is a digital pwm write). Digital write .. and analog read.

The first 2 are for digital pins, the 3rd is for an analog pin.

So there is no ambiguity when using these methods because they are specific to a pin type.

So, I'm not quite sure what I'm recommending here, sorry. I need to do a little more research to make sure I'm recommending something good.

At the end of day knowing that pin 14 magically maps to A0 on the info.. but if you have a mega.. pin 14 is a digital pin... This is confusing and should be sorted.

So far I think

public int Arduino.D0  = 0;
...
public int Arduino.D52  = 52;
public int Arduino.D53  = 53;

Where the values may get changed depending on the setBoard(type) methods is what I favor at the moment.

I'm guessing this is how its basically done in the Arduino firmware ...   a tangle of #defines which depending on board slelection make D7 "do the right thing"

PinDefinition should be the class which contains all the details about a pin ..  I think this is a good design, and it can satistfy requirements from other boards, not just Arduino.

But what I don't want happening (again) is PinDefinition stuff being read back with the data.  
In the old days, a pinType would come back with the pin to tell if its analog or digital or whatever ..  this is not needed nor wanted.  The unique identifier should be the pins address. And with that address you can get the PinDefinition "after" you read it back.

I think this can still be clear reading it back like this.

e.g.

def onPinArray(pinArray):
     for pinData in pinArray:
             if (pinData.address == Arduino.A5):
                        pirTriggered()

 

I wrote the above forgetting the notation of Arduino.A5 means its a static.

So changing these values will interfere with other Arduinos which do not have overlap.  Uno overlaps Mega so the values could be constant in that case.  But Mat knows these values have to change for boards, which ones?

We could have them change per instance - notation would be much less desirable arduino.A5 
Which I don't want to do.

Hmmmm....

So, the only thing I don't like about  using  Arduino.A5 for example, is that you'd have to an import statement at the top of your python script to use it.  I suspect people won't know to do that, though we could have documentation / examples that show how to do it maybe that would alleviate my concern.

I guess at the end of the day,  the real issue is that the arduino library will use a different #def for the pins depending on which board you're on (uno, mega, other.) 

So, it's a little difficult for us to map from java land to what the compiled pin number is in mrlcomm land.