ev3dev / ev3dev-lang

(deprecated) language bindings for ev3dev sensors, motors, LEDs, etc.
GNU General Public License v2.0
56 stars 39 forks source link

Changes to support new tacho driver updates #152

Closed rhempel closed 8 years ago

rhempel commented 8 years ago
jabrena commented 8 years ago

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

WasabiFan commented 8 years ago

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

rhempel commented 8 years ago

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

rhempel commented 8 years ago

@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 commented 8 years ago

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

rhempel commented 8 years ago

@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 commented 8 years ago

That makes sense. Feel free to make that change.

dlech commented 8 years ago

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 commented 8 years ago

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 commented 8 years ago

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 commented 8 years ago

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 commented 8 years ago

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

rhempel commented 8 years ago

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 commented 8 years ago

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

dlech commented 8 years ago

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

WasabiFan commented 8 years ago

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

rhempel commented 8 years ago

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

WasabiFan commented 8 years ago

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 commented 8 years ago

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 commented 8 years ago

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 commented 8 years ago

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 commented 8 years ago

:+1:

rhempel commented 8 years ago

@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 commented 8 years ago

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 commented 8 years ago

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 commented 8 years ago

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

dlech commented 8 years ago

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

dlech commented 8 years ago

Additional changes needed are

rhempel commented 8 years ago

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.