Conversation on language binding spec

[WasabiFan]

I have almost completed a preliminary draft of the spec for motors and sensors. It can be found here: https://github.com/fdetro/ev3dev-lang/blob/language-binding-unification/wrapper-specification.md.

For this first version, I tried to mostly do direct mappings of sysfs properties to object properties. I am looking for feedback on what I have written so far, and I also have a few questions about finishing the feature set so that we can get to a version that we can implement.

Here are my questions:

• Enums. There are some values, such as 'on' and 'off', or 'A' through 'D' that have a specific set of values. Instead of strings, we could accept enums (in the languages that support them) for properties like this. Do we want to do this?
• Device initialization. What should the constructor for motors and sensors take? I would vote for the external port (A through D), and have the wrapper find the correct device index. Would there be a reason that that would be a bad idea?

Once we answer these questions and tidy up the spec, we should be able to start implementing this in code.

[fdetro]

Re enums: in general I would like to stay with the native sysfs types, so 'on' and 'off' as strings are fine. Regarding the port IDs (1 .. 4 for inputs, A .. D for outputs I would prefer numbers for the parameters, but unnamed enums as helper (INPUT_1 = 1, INPUT_2, .. OUTPUT_A = 1, OUTPUT_B = 2, ...).

The value '0' for ports I would like to be interpret as 'Don't care', as in my implementation.

This leads to re: Device initialization: I agree with your approach, but with '0' as 'Don't care'. In this case, we take the first matching device that we find (this is my current implementation).

Another remark to sensors: I would like to have two getValue members, one for the raw value and one for the scaled (dp) value. Return type for the raw data should be int, scaled data should be float.

Does that seem reasonable?

[WasabiFan]

So, 0 would mean "find the first motor that is plugged in" for motors, and "find the first sensor that is plugged in" for generic sensors? If so, would that also mean that specific types of sensors would store the list of sensor IDs that they can handle, and use that to narrow the search? Or would the device type be an argument? I personally like the first option better, but we could probably combine the two with a separate function.

I'll add this to the spec, and we can see how it looks.

Separate topic: Versioning

I think we are getting close to the point where we can start adapting our existing code around our spec. That means that we will need to figure out a versioning system. Do we want to have all of our packages maintain the same version number (along with the spec)? I'm thinking we have them all start out at 1 or 0.9, because the current version of my published NodeJS package is 0.31, and NPM doesn't let me go down in version numbers.

[WasabiFan]

I have made the discussed changes to the specification.

[fdetro]

Re versioning: 0.9 is ok for me.

Re error handling during initialization: in C++, throwing exceptions from constructors is pretty problematic, we should avoid this. In my implementation motors and sensors have a member connected, which can be queried after constructing the object. What do you think?

Regarding the device enumeration discussion, we should probably first think about the sensor class hierarchy. For which sensors do we want to have special classes? Should we introduce base classes for similar sensors?

Re: getRawValue vs. getValue: as only some values do have a dp != 0 / a float representation I would prefer to have getValue for the raw value and getFloatValue or getValueAsFloat for the float value. OK?

[WasabiFan]

I have added the version info and error info to the spec, and I renamed those two methods.

For a start, I was thinking these sensors would be good to have special classes for:

• Touch Sensor
• Ultrasonic Sensor (just EV3 for now, NXT units are different and would require a special case)
• Gyro (EV3)
• Color? (EV3)

Other than maybe the ultrasonic sensor (where the EV3 and NXT versions are very similar but slightly different), I don't think we need base classes other than Sensor. That would introduce extra complexity that probably wouldn't provide much benefit.

How do you want to handle special sensor classes? For example, the color sensor has modes for RGB, color index, and reflection. Would there be a property for each (R, G, B, Color Index, Reflected), and then getting each value would change the mode to what is needed automatically? Or would it throw an error if it wasn't on the correct mode, and have the user change it?

Tonight, I am going to bring in my Node JS binding and start gutting it so I can rebuild it for the new spec when it's done.

[fdetro]

Other than maybe the ultrasonic sensor (where the EV3 and NXT versions are very similar but slightly different), I don't think we need base classes other than Sensor.

In my implementation I have another class msensor as there are also the older I2C/S sensors which are not modeled inside of the msensor sysfs tree.
That would introduce extra complexity that probably wouldn't provide much benefit.

How do you want to handle special sensor classes? For example, the color sensor has modes for RGB, color index, and reflection. Would there be a property for each (R, G, B, Color Index, Reflected), and then getting each value would change the mode to what is needed automatically? Or would it throw an error if it wasn't on the correct mode, and have the user change it?

From efficiency point of view I do not like any of these alternatives. In any case we would need a string compare on the mode for every value access :-(

[WasabiFan]

I think support for non-mindstorms I2C sensors should probably wait for a later release. Managing drivers manually will take a lot of custom code in many languages, something that would probably greatly delay an 0.9 release. I2C/M sensors are auto detected and loaded in to the msensor node, so we should have fairly good coverage of the sensors without specific support for I2C/S, which we can add later.

From efficiency point of view I do not like any of these alternatives. In any case we would need a string compare on the mode for every value access :-(

Hmmmm... good point. I guess our options then are either to just get the index that the value should be at (and if it's the wrong mode, that is their problem), or have an option to enable/disable the checks (which would still require a comparison, but Boolean comparisons are fairly fast). Something like a "safe mode"? I think that would give a fairly good balance of performance and error-checking/ease-of-use. Or, maybe we could convert the mode to an index (as listed in the modes attribute) whenever the mode is changed, and compare that, to make it a bit faster?

[topikachu]

My classes

MSensor=> lego.TouchSensor, lego.ColorSensor,  ... all Msensor
I2CS=> mindsensors.MindSensorI2CS => mindsensors.PSPNxV4, mindsensors.AbsoluteIMU
Motor=>  lego.LargeMotor, lego.MediumMotor

I suggest giving a base class for non-mindstorms I2C to encapsulate low level I2c operations. I use the smbus binding for python.

[topikachu]

Do you have implemented the "0" port? if not, I suggest using "-1". Because "0" is really confused for user to remember if the port index is 0 based or 1 based.
But "-1", for sure, is not a valid index.

My 2 cents

[fdetro]

Do you have implemented the "0" port? if not, I suggest using "-1". Because "0" is really confused for user to remember if the port index is 0 based or 1 based.

There are enums / constants for the ports, so I do not see a problem here. I want to reserve negative port numbers for future extensions like "take the second connected sensor".

[WasabiFan]

Using -1 could work well with negatives being "find the nth available device". 1-4 would be the actual ports, -1 - -4 would do what you described above. Although in the end, as long as people are using the enums, it shouldn't matter.

Anyway, I just committed a small change to the spec that defines a reset() method. I found that that is a clean way to make sure that the motor will do what you think it will. I am working on a project using the new version of the NodeJS binding right now so that I can find any bugs; I will be adding my code to this repo soon.

[fdetro]

When converting my code to the new spec I realize that both sensors and motors do lack an initialization parameter: type. In case of sensors this is the lego sensor type ID (number), in case of sensors this is a string. Do you agree?

[WasabiFan]

Yes; that makes sense. But can we say that it is optional (in all languages that support optional parameters)? One might want to be able to just allow all sensor or motor types, like if they are running a test or don't care what sensor it is.

Past that, someone might want to allow multiple sensors (for example, the EV3 or NXT version). So maybe the parameter should be an array?

[fdetro]

At the moment, in my implementation 0 (as sensor type) / an empty string (as motor type) does mean "don't care". I like your idea with a list of allowed types. This is easily realizable in lua and c++.

[WasabiFan]

Ok; I will write this up in the spec.

[WasabiFan]

Sorry it took so long, but I have finally implemented sensors in the JS binding.
What's the status of the Lua and C++ libraries? I'll be adding sensors to my Vala binding soon. After I get that done, and assuming that the C++/Lua ones are ready too, I would imagine that we could tag and release the 0.9 version, and move on to working on a v1, which would probably include things like LEDs and more advanced sensor support.

I will be closing up my old separate repo and redirecting people over here in the coming hours, just so you are aware. By the way, thanks again for letting us use your repo for our combined library efforts.

[WasabiFan]

@fdetro What do you want to do about the branches? Maybe set language-binding-unification as the main GitHub branch? Or would you like to keep it how it is right now?

[WasabiFan]

Excerpt from ev3dev/ev3dev.github.io#14:

I you guys want to move the language unification repo under the ev3dev organization, we can do that too.

[fdetro]

I have merged the changes from master into the language-binding-unification branch. If you have completed your changes we can merge language-binding-unification back into master and create the 0.9 release.

For me it's OK to move the repo under the ev3dev organization.

[WasabiFan]

My JS binding is ready for a release, and my Vala binding is close. I'll finish up there and then we should be ready.

@dlech, if we move this over to the ev3dev org, we can still have push access, right? If so, I think that will work out nicely.

[dlech]

Yes, we can create a "language bindings team" and give the 3 of you write access. Then you can approve my pull requests for a change 😄

[dlech]

I just created this group and sent invites. @fdetro should be able to go into the settings for this repo and select transfer ownership and transfer it to the ev3dev organization. GitHub will automatically create redirect links to the new location. Or we can create a new repo under ev3dev and you can push there and leave this repo for posterity.

[dlech]

Also, here are some things to consider... I have started issue ev3dev/ev3dev#143 to discuss some of these things further if you would like to have any input.

• Port names: A Motor may not necessarily be plugged into an output port and sensor may not necessarily be plugged into an input port. Also, there may be more than 1 motor or sensor per port.
  • Case 1: There are a number of motor control devices that actually plug into input ports (and use external power supplies) meaning port_name would return "in1" even though it is a motor.
  • Case 2: There may be more than one motor or sensor per port (multiplexer). mindsensors.com motor multiplexer is example of cases 1 and 2. In this case, we would probably return "in1:1" and "in1:2" for the port names or have 2 different attributes that you need to look at.
  • Case 3: Multiple I2C sensors can be connected to the same port as long as they have different addresses, so port name might not be enough to identify which sensor we are trying to communicate with.
  • Case 4: The motor or sensor may not be plugged into an input or output port at all. Example: http://lechnology.com/2014/09/using-uart-sensors-on-any-linux/. In this case, we are currently returning the name of the underlying device for port_name. So for my USB example, I get "ttyUSB0". This is not ideal because it does not uniquely identify the sensor if I have more than one USB sensor. I will have to think on this some more.
• Types of motors: ev3dev will support at least 3 different motor classes.
  • tacho-motor: This is the motor we currently have implemented. It is characterized by the fact that it has a shaft encoder that provides positional and directional feedback to the EV3.
  • servo-motors: These are the hobby type servo motors that just turn 90 degrees in either direction (although there is a continuous rotation variant). These will show up under /sys/class/servo_motor. Someone has donated one of these, so I will be adding support for this soon.
    *motor: Just a plain motor, like the LEGO Power Function motors. We will be able to control speed and direction on these, but there is no feedback so they will have their own class.

[fdetro]

I have moved the repo into the ev3dev organization. Thanks to @dlech.

[fdetro]

Re @dlech on port names and sensor / motor multiplexing: we should also consider a USB daisy chaining as implemented in the stock LEGO ev3 image.

But first of all we need to get the basic functionality working, next steps are unified APIs for LEDs, buttons, battery etc.

[dlech]

For daisy chaining, I am thinking that we just implement a remote control protocol over TCP/IP, then you can have USB, Bluetooth or WiFi "daisy chaining". In this case, the port names could be <ip-address>:in2.

[WasabiFan]

@dlech the issue I have with using TCP is that it can be fairly slow. I tried reporting Xbox 🎮 controller values through the USB networking connection, and only 40 bytes/sec was enough that the ev3 slowed and crashed. I would prefer that we implement LEGO's proprietary protocol. If we can send and receive those same commands, then one could use an ev3dev EV3 to control a stock ev3 (which I did in a custom program; its pretty cool). You can get an idea of how the protocol works in the stock kernel source. Basically, if the port number addressed is out of the hardware range, it propagates it to the next device in the line.

[WasabiFan]

@fdetro I have finished and fully tested my vala/js bindings. They are both fully compliant with v0.9 of the spec, and I tested all the basic functionality in both. Assuming the other two are good-to-go, I think we can tag this and make a release. Sound good?

[fdetro]

Sound good. I have merged the changes from the branch to master and drafted a release v0.9.0. Feel free to review / extend the comment and finalize the release.

[fdetro]

Should we now proceed with LEDs and battery? These should be easy, buttons will need some more thinking regarding synchronous vs. asynchronous reads.

[WasabiFan]

I think so; can I create a GitHub release for our current code, and push my Node module to NPM? Once we do that, I think starting on LEDs and battery (for maybe a 0.9.5 release?) would be good.

[WasabiFan]

Never mind... didn't see your other comment. Tagging and releasing now!

[WasabiFan]

Should I also delete the separate branch so that we work off of master? Or keep the separate one for dev work, and only merge to master on release?

[dlech]

Should I also delete the separate branch so that we work off of master?

From my experience working on GitHub, I would suggest working on master if you want other people to be able to follow along. Other branches stay kind of hidden from view unless you go looking for them. So, I guess it just depends on how much you want people (like me) poking their noses in your business. 😄.

buttons will need some more thinking regarding synchronous vs. asynchronous reads.

Switching my philosophy from "cover every possibility" to "keep it simple", I would suggest having 2 functions for buttons based on what people are probably used to from EV3-G and NXT. Have one function that reads the current state of a button and a second function that blocks until a button has a certain state (pressed or released).

[fdetro]

Should I also delete the separate branch so that we work off of master?

I would prefer to continue major rework on branches, so let’s stay on the language-binding-unification branch for now. OK?=

[fdetro]

Have one function that reads the current state of a button and a second function that blocks until a button has a certain state (pressed or released).

I agree on the function which checks the current state of a single button, but probably the blocking one should deliver a bit set or maybe even some kind of events?=

[rhempel]

Agree with @dlech, need both asynchronous and blocking button read capability.
As far as working on GitHub, please continue to use branches for rework and testing, and when you feel confident about general release merge in to the master branch. That's how I handled the polarity-mode fixes. Use the github repo as a safety net for your local repo, then when you're all merged and done, you can delete the branch on the master too.
Keep the work on a branch small and focused if possible.

[WasabiFan]

The binding branch is currently set to show up by default on the GitHub page, so I think that makes it easier for people to find.

[dlech]

Sorry, I used the wrong terminology. I meant master == GitHub default branch rather than master == branch named master.

[fdetro]

The binding branch is currently set to show up by default on the GitHub page, so I think that makes it easier for people to find.

I have merged the binding branch back to master and also prepared the release on master. It is common git practice to have the stable code on master and doing the work on other branches.

Do you know the git flow model?

http://nvie.com/posts/a-successful-git-branching-model/
http://yakiloo.com/getting-started-git-flow/

[WasabiFan]

@fdetro Yep; I'm good to work off of the current binding branch.

[WasabiFan]

@fdetro I was just saying that having the GitHub "main" branch (the one that shows up when you first open the repo) be the work branch (language-binding-unification) makes it more obvious for people to find, because most people that come here will be looking for the latest, not the most stable (at least for the time being). This also turns out to be what I think @dlech was saying.

[WasabiFan]

I think this issue worked well for conversing about the initial work, but now we're at the stage that there are specific features that we are looking to implement. Can we close this issue and use dedicated ones from here on out to discuss the new features that we will be wanting to implement?

[WasabiFan]

Just to finish up the branching conversation, how do we want to operate? Currently, we have the main branch sitting as the repo was when we finished v0.9, and some small changes committed in to the language-binding-unification branch. This latter branch is set as the default branch that shows up on GitHub.

I'm thinking we rename the current master branch to something like v0.9, and rename language-binding-unification to WIP or vNext. Then we can do work on the work branch and branch it to a version-ed branch when we release. What do you think?

[rhempel]

This starts the conversation around what "master" is. Is it the last shpped version, and the branches/tags are works in progress, or is the next version we'll ship, and you go back to a tag to get a specific shiiped version.
Feature development and bug fixes should always happen in their own branches regardless, and be merged once they are tested/verified.
Open to suggestions, but let's decide on the meaning of master first :-)

[dlech]

Short version:

Is it the last shipped version, and the branches/tags are works in progress

No.

or is it the next version we'll ship, and you go back to a tag to get a specific shipped version

Yes.

---

Long version:

I put @fdetro and @WasabiFan in charge of this particular repo, so as far as I am concerned, they can decide to manage it how they like. 😄

For reference though, most projects on GitHub use master as the main development branch. If releases get bug fixes separate from the main development branch, then the project probably has separate branches for releases. If not, releases are just marked by a tag with no branch. There is no point in having a release branch unless there are extra commits not included in master that cause it to "branch" off.

Also, generally speaking on GitHub, feature development branches usually happen in the users fork of the main repo rather than in the main repo itself. The exception would be if multiple core contributors were collaborating on the same feature at the same time.

In the case of ev3dev, most of our main development branches are called ev3dev-jessie. The reason for using this rather than master is because most of the repos are Debian packages. The Debian packaging system is integrated with git and this makes things work out better since there are upstream branches that in many cases use the name master. So, if you want to be consistent with the rest of ev3dev, call the main development branch master or if you add Debian packaging, call it ev3dev-jessie.

[WasabiFan]

@dlech That makes sense. I'd say we call it master, because the repo probably won't be packaged as a whole. Node libraries, for one, are usually shared via NPM, and they have a good system in place for globally installing Node JS modules like one would with a package manager. Past that, the other three can be packaged, but they should probably be packaged separately. (There's also a discussion there about possibly including some of these in ev3dev at a later date, but we first need to get some features implemented).

As for the branching/tagging, I'd say we branch it off at every release. Maybe we adopt a naming convention (something like release/v0.9 for releases, feature/LCD for features) to keep it organized, but generally I like to be able to isolate the different releases so that we can merge in bug fixes if need be. Whether we put feature branches in the main repo or in our own forks should just be a matter of the scale of the feature; if it's a tiny change, it probably doesn't even need a branch.

[rhempel]

WHen you say branch in this context, I think you mean tag. release/v0.9 is really just a tag made from the master or development branch that represents what files went into 0.9 when it was bundled up. Always, always, always tag as a pre-build tag, then build from that tag as a final test, and when it all checks our give it a final tag.

feature/LCD is a branch on your local repo for when you're working on that feature, you merge it into master when it's ready on your side.

[fdetro]

I still vote for using a standard git / github model. master is used for publishing the stable state of the project (typically a sequence of releases), other branches are used for development / features / preparation of releases. A release means merging a new stable state into master and tagging this state.

If a fix is needed on the current release, it is merged to master and there will be a new tag (e.g. v0.9.1). If a fix is needed for an older release, we can think of creating maintenance branches.

Does this sound reasonable?

[WasabiFan]

@fdetro Sure, I'd be fine with doing it like that. So we would keep the current "master" as-is ( and tag releases there), and then rename "language-binding-unification" to something like "development"? Is it O.K. with you if we keep the development branch as the 'default' on GitHub, so that it shows up when you load the page?

Assuming that @dlech and @rhempel are fine with this system, @fdetro feel free to rename the branches as you see fit (I'll back whatever names you chose).

[rhempel]

I think @fdetro is asking for tags to be created on master and a new branch is taken out for any development - then master is always the last shipped release? Right?

[WasabiFan]

Yes, I think that is what @fdetro is saying.

P.S. Upon further reflection, I realized that I hadn't written the same thing that I was thinking, so I updated my comment above, in case that was what you were noticing. Sorry for adding more confusion!

[fdetro]

OK, then I will rename the language-binding-unification branch to develop, and we keep develop as the default branch in github, OK?

This will allow me to use the gitflow model for my future development.

[WasabiFan]

@fdetro Sounds good to me.

[fdetro]

Done.