Changes to support new tacho driver updates #152

rhempel · 2016-03-06T21:25:01Z

Eliminate speed_regulation_enabled attribute
Add max_speed attribute
Add count_per_m attribute for linear motors
Add fullt_travel_count attribute linear motors
Add subclasses for all known motor types
Change the speed_regulation_pid parameters to speed_pid
Add a paragraph to explain when attributes are handled.
See Can't change speed_sp anymore while motor is running ev3dev#558
Fix line endings without commas

- Eliminate speed_regulation_enabled attribute - Add max_speed attribute - Add count_per_m attribute for linear motors - Add fullt_travel_count attribute linear motors - Add subclasses for all known motor types - Change the speed_regulation_pid parameters to speed_pid - Add a paragraph to explain when attributes are handled. - See ev3dev/ev3dev#558 - Fix line endings without commas

WasabiFan · 2016-03-06T21:29:07Z

spec.json

@@ -362,7 +359,40 @@
            ],
            "inheritance": "motor"
        },
-        "dcMotor": {
+        "nxtMotor": {


I think we should do something like we did for specialSensorTypes for these, because they should be able to be completely automatically generated.

See https://github.com/ev3dev/ev3dev.github.io/blob/master/_data/motors.json

WasabiFan · 2016-03-06T21:35:55Z

spec.json

                ],
            "docsLink": "http://www.ev3dev.org/docs/drivers/tacho-motor-class/",
            "systemClassName":  "tacho-motor",
-            "systemDeviceNameConvention":  "motor{0}",
+            "systemDeviceNameConvention":  "*",


What's this?

It's a workaround for the fact that we have two types of motors - the Firgellis have a device name of linear* while all others are motor* - if you instantiate using ev3dev.Motor and then change the driver type to a linear motor, the device name changes to linear*.

You can always instantiate the right motor type and get the more precise motor name

ddemidov · 2016-03-07T06:26:13Z

spec.json

@@ -344,6 +346,7 @@
        },
        "largeMotor": {
            "friendlyName": "Large Motor",
+            "systemDeviceNameConvention": "motor{0}",


Not really related to this PR, but looks like we have a mix of two mask styles: * and {0}. May be we need to choose one.

I agree -- we should normalize on a way of specifying this information and stick with it. I'll need to take a look at the underlying device code in my JS lib to refresh my memory on how I do it, but I think I currently transform it into a regex. That would probably be a good format to use.

In python this gets translated to file system mask, so * may be used directly. {0} is also not a problem though, because it coincidentally is the string format marker in python (as in 'motor{0}'.format('*')).

@WasabiFan - I will defer to your judgement on this - I agree with pick one and stick with it

I say we normalize to * -- it's a universally understood symbol for a wildcard and single characters are simpler in general.

jabrena · 2016-03-07T07:12:07Z

Good morning, how many weeks to release a new version to be effect the changes in motor class?

WasabiFan · 2016-03-07T14:18:19Z

@rhempel Can you update the spec version number and supported kernel version? I think the spec version deserves incrementing the major.

rhempel · 2016-03-07T17:46:42Z

Will do - chicken and egg - I need to know also the next kernel cycle version :-)

rhempel · 2016-03-08T01:08:22Z

@WasabiFan - the version in my branch looks like this:

    "meta": {
        "version": "0.9.3-pre",
        "specRevision": 2,
        "supportedKernel": "v3.16.7-ckt16-7-ev3dev-ev3"

The version in develop looks like this:

    "meta": {
        "version": "1.0.0",
        "supportedKernel": "v3.16.7-ckt21-9-ev3dev"

The proposed new version will be

    "meta": {
        "version": "1.0.0",
        "supportedKernel": "v3.16.7-ckt21-10-ev3dev"

And if we do a non-breaking revision after that, we just add a

        "specRevision": ?,

line, OK?

WasabiFan · 2016-03-08T02:04:55Z

As long as we plan to tag an ev3dev-lang release after these changes are released, I'm fine with that.

rhempel · 2016-03-08T12:31:12Z

@WasabiFan,I was thinking about this after my discussion with @dlech and probably the right thing to do is make the supportedKernel a regex, like this:

"supportedKernel": ".*-10-.*-ev3dev"

That way it's independent of the target board kernel and depends only on our updates to the drivers...

WasabiFan · 2016-03-08T13:54:19Z

That makes sense. Feel free to make that change.

dlech · 2016-03-08T18:40:46Z

Regex would actually be [\w\-\.]+\-10\-ev3dev-[\w\-\.]+

Anything between the 10 and ev3dev is for development releases only and can probably be omitted.

rhempel · 2016-03-09T00:32:15Z

I thought the kernel version string had to end in -ev3dev to be recognized as an ev3dev kernel?

And this re does match:

print(re.match('.*\-10\-ev3dev.*', "v3.16.7-ckt21-10-ev3dev"))

rhempel · 2016-03-09T00:49:36Z

Here's a slightly better regex that actually matches the kernel string I have:

import re
print(re.match('[\w\-\.]+\-10\-[\w\-\.]*ev3dev[\w\-\.]*', '3.16.7-ckt21-10-rc1-ev3dev'))
print(re.match('[\w\-\.]+\-10\-[\w\-\.]*ev3dev[\w\-\.]*', '3.16.7-ckt21-10-rc1-ev3dev-ev3-rhempel-ev3+'))

dlech · 2016-03-09T00:50:02Z

Nope, EV3 hardware ends in -ev3, RPi1 hardware ends in -rpi, RPi2 hardware ends in -rpi2 and Beaglebone ends in -bb.org. These are called kernel "flavors" and are used by the flash-kernel utility to indicate that a kernel is installable on a particular hardware platform.

The ev3dev is just for us human beings.

dlech · 2016-03-09T00:51:26Z

Does this regex have to actually match against uname -r on a running machine?

rhempel · 2016-03-09T01:00:40Z

No, it does not have to match, but it's a clue that you can use to figure out why your script isn't working when we change attributes :-)

rhempel · 2016-03-09T01:01:38Z

I was just trying to figure out some way to make that string useful across all the kernel strings that we might support

dlech · 2016-03-09T01:03:33Z

If it is not used programmatically, then why not make it human readable like <upstream version>-10-ev3dev-<flavor>?

WasabiFan · 2016-03-09T01:06:15Z

Honestly, I think it would make sense to add an isPlatformSupported function that would use this regex to compare against the kernel version.

rhempel · 2016-03-09T03:02:57Z

What if the kernel verison changed with no changes required for spec.json?

WasabiFan · 2016-03-09T03:20:13Z

Then the support method returns false, saying that it is an untested platform. If we have tested it and it is supported, we publish a new version.

rhempel · 2016-03-09T12:36:02Z

I'm in favour now of @dlech's suggestion of a human readable expression - the language binding can easily replace the known text with the appropriate matching pattern in case it's not compatible with standard regexen. I'll add a section for optional release candidate like this:

<upstream version>-10-<optional text>ev3dev-<flavor>

unless we can convince @dlech to make <optional text> like rc1 part of the <flavor> string :-)

dlech · 2016-03-09T18:08:11Z

There will never be an official release with -rcX or any other "optional text". Only if you build your own kernel from git will you have -rcX. And technically, -rcX and a few other strings are treated specially in kernel versions. So, the -rcX is actually part of the number 10. As in 10-rc1 < 10-rc2 < 10.

So, no, I can't move "optional text" because it would change how version numbers are compared and it doesn't seem appropriate to have it in the spec anyway unless the spec is targeting a pre-release version, in which case you would specify 10-rc1 explicitly.

rhempel · 2016-03-09T18:21:27Z

OK, how about <upstream version>-<ev3dev version>-ev3dev-<flavor> - that way we can handle release candidates during development is we have a function like @WasabiFan suggests to check kernel compatibility.

And we can have a list of compatible s in the meta data.

Sorry if I'm dragging this out, but I deal with checking hardware and firmware compatibility regularly at work, and it's a mess if you get it wrong.

dlech · 2016-03-09T18:44:07Z

👍

- Added an easily matched text string to allow for a more complex regexp to be used to match kernel variants across supported platform - Added a list of kernel variants that are compatible with the current json.spec

rhempel · 2016-03-10T23:21:37Z

@WasabiFan and @dlech, I've added the support for allowing kernel compatibility to be checked. The strings are easily identified to allow for binding-specific string matching, and we have an array of compatible kernel variants that the binding can iterate through to test for compatibility :-)

WasabiFan · 2016-03-11T00:06:41Z

I'm not sure that I understand. What's the point of the pattern if we also have an array of compatible kernel strings?

rhempel · 2016-03-11T00:16:08Z

The pattern is there because it lets the binding easily substitute patterns for the upstream version and the flavour, and iterate through the supported kernel versions. I see your point about just listing supported kernel variant strings. The patterns allow the binding to know where to find the important xx-yyy-ev3dev in the kernel name in a language independent way.

This becomes important when we support different upstream kernels and the flavours include EV3, rPi, BB, etc

WasabiFan · 2016-03-25T01:34:52Z

What's the plan for this? Merge it once the new drivers are released?

dlech · 2016-04-12T15:29:38Z

Drivers have been released so you can proceed with this pull request.

dlech · 2016-04-12T15:35:12Z

Additional changes needed are

Removal of encoder_polarity attribute (tacho motor class)
Rename of stop_command to stop_action (both tacho motor class and dc motor class)
Rename of stop_commands to stop_actions (both tacho motor class and dc motor class)

rhempel · 2016-05-08T02:13:33Z

I'm going to merge this as-is, and open a new PR for the breaking changes to the new kernel that @dlech mentions above.

WasabiFan reviewed Mar 6, 2016
View reviewed changes

WasabiFan added enhancement all languages labels Mar 6, 2016

WasabiFan reviewed Mar 6, 2016
View reviewed changes

Add missing full_travel_count attribute

a63978f

ddemidov reviewed Mar 7, 2016
View reviewed changes

Add support for compatible kernel detection

cdb0250

- Added an easily matched text string to allow for a more complex regexp to be used to match kernel variants across supported platform - Added a list of kernel variants that are compatible with the current json.spec

Merge branch 'develop' into feature-new-tacho-driver-attributes

8f4b6a9

WasabiFan mentioned this pull request Mar 26, 2016

Updating human-readable spec #146

Open

4 tasks

Reapply changes that were lost in a bad merge

9c9e8d7

rhempel merged commit 20d4401 into develop May 8, 2016

WasabiFan deleted the feature-new-tacho-driver-attributes branch September 15, 2016 01:31

Changes to support new tacho driver updates #152

Changes to support new tacho driver updates #152

Uh oh!

Conversation

rhempel commented Mar 6, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jabrena commented Mar 7, 2016

Uh oh!

WasabiFan commented Mar 7, 2016

Uh oh!

rhempel commented Mar 7, 2016

Uh oh!

rhempel commented Mar 8, 2016

Uh oh!

WasabiFan commented Mar 8, 2016

Uh oh!

rhempel commented Mar 8, 2016

Uh oh!

WasabiFan commented Mar 8, 2016

Uh oh!

dlech commented Mar 8, 2016

Uh oh!

rhempel commented Mar 9, 2016

Uh oh!

rhempel commented Mar 9, 2016

Uh oh!

dlech commented Mar 9, 2016

Uh oh!

dlech commented Mar 9, 2016

Uh oh!

rhempel commented Mar 9, 2016

Uh oh!

rhempel commented Mar 9, 2016

Uh oh!

dlech commented Mar 9, 2016

Uh oh!

WasabiFan commented Mar 9, 2016

Uh oh!

rhempel commented Mar 9, 2016

Uh oh!

WasabiFan commented Mar 9, 2016

Uh oh!

rhempel commented Mar 9, 2016

Uh oh!

dlech commented Mar 9, 2016

Uh oh!

rhempel commented Mar 9, 2016

Uh oh!

dlech commented Mar 9, 2016

Uh oh!

rhempel commented Mar 10, 2016

Uh oh!

WasabiFan commented Mar 11, 2016

Uh oh!

rhempel commented Mar 11, 2016

Uh oh!

WasabiFan commented Mar 25, 2016

Uh oh!

dlech commented Apr 12, 2016

Uh oh!

dlech commented Apr 12, 2016

Uh oh!