Add autogen spec for special sensor types

[WasabiFan]

This is an initial test of the ideas discussed in #126. I just added the touch sensor for now, and after we get the specifics of the spec structure ironed out I can add more to this PR.

I basically just moved the TouchSensor class from the classes section to a new specialSensorTypes section, and then added the details of the isPressed property. I decided to move it to its own section because it makes it easy to distinguish between the main classes and these special ones in an autogen script, although we can probably put it in classes instead if desired.

The isPressed property is described similarly to other properties, except instead of using a systemName is uses a sourceValue number corresponding to one of the valueX attributes.

Does this seem like the right track? Thoughts?

@ddemidov

CC @dlech

[ddemidov]

I basically just moved the TouchSensor class from the classes section to a new specialSensorTypes section

I am not sure I like this. As I understand, the plan is to provide all the sensor classes with these kind of helper functions. So will they all migrate to special sensor section eventually? We could just leave touch sensor where it is now, since specialValueMappings is new subsection and would be ignored by existing templates.

[WasabiFan]

My main goal there was to make it easy to selectively autogen just those ones. My current JS bindings don't autogen the class itself, just the properties and methods -- so if they're mixed in I will need to selectively filter them out. That shouldn't be too difficult though, and longer-term I think it makes more sense to keep them together, so I'll move it back into the main classes section.

[WasabiFan]

How's the rest of the spec data/structure? Do you think that there's anything else that we need to have in there?

[ddemidov]

so if they're mixed in I will need to selectively filter them out.

I don't understand this. Other properties are in systemProperties section, and systemProperties and specialValueMappings will require different templates anyway. At what step would you need filtering?

[WasabiFan]

At what step would you need filtering?

I'll need a loop to iterate over the classes in the classes section and only generate these special sensor classes for items with the added properties. This is (hopefully) only temporary however as I am working to attempt to get my system set up to autogen all these classes.

[ddemidov]

How's the rest of the spec data/structure?

The rest looks fine to me, but I did not yet try to use the spec for generation. What slightly bothers me is what if we need some kind of compound property. For example, we could return a tuple/struct for RGB color mode of color sensor. This could be implemented in python as

return (value0, value1, value2)

But I don't think we can make a specification for this that works across all languages without creating a mini language with a parser, so we could just get away with red = value0, green = value1, red = value2 helper properties.

[WasabiFan]

I was trying to think through the list of sensors that we need to support to see if there are any that need properties that blatantly won't work with this system, and it seems like as long as we're willing to go with separate properties for compound values like that we should be fine. Of course, we can manually implement the compound ones which call on the individual ones if we want to.

[ddemidov]

I'll need a loop to iterate over the classes

I looked at js bindings, and it seems you do the same thing I do:

class some_sensor {

// autogen get_properties
// autogen
}

Could not this just become extended to

class some_sensor {

// autogen get_properties
...
// autogen

// autogen get_special_properties
// autogen
}

The special properties section would be just empty for classes that don't have it.

[WasabiFan]

The special properties section would be just empty for classes that don't have it.

My hope is that these sensor classes are simple enough that the entirety of the class can be generated instead of the individual parts as I do with the other classes. To do this, I need to distinguish between the sensor helper classes and all the others. But I think this really is a non-issue for me because we have full control of the helper methods which our templates have access to, so anything that can't be done natively in Liquid can be implemented in the codebehind of the script in only a line or two.

[ddemidov]

Does that mean touch sensor class (and all that follow) would not have access to raw value* functions? I though that we would leave those in, so users could still use them if they wanted to get rid of mode switching overhead in some performance sensitive cases.

[WasabiFan]

Does that mean touch sensor class (and all that follow) would not have access to raw value* functions?

With my current plan, these sensor classes will be descendants of the base Sensor class, so they should have all the accessors that other sensors do including the value* ones. I'm just saying that I plan to have a template that makes this:

/** 
 * Other sensor
 */
export class MyOtherSensor extends Sensor {
    constructor(port?: string, driverNames?: string[]) {
        super(port, ["one-driver-name"]);
    }

    get oneExtraProperty() {
        this.mode = 'foo-mode';
        return this.getFloatValue(0);
    }

}

[ddemidov]

Ok, I understand the plan now. But having two versions of the same sensor class does not sound right to me. Would the base class be hidden from the user? What if it does not have special properties? How would you name the base and the derived classes? Basically, I think it would be more straightforward to just have one class with or without helper methods.

[WasabiFan]

Basically, I think it would be more straightforward to just have one class with or without helper methods.

OK... now I'm confused :wink: Are you saying you don't think we should have the normal Sensor class anymore? I doubt that's what you're saying, so there is probably some deeper miscommunication going on.

having two versions of the same sensor class does not sound right to me.

I agree; I don't expect to need (and don't want to have) two versions of the same class. I would keep the normal sensor class just as it is now, and then generate a class which inherits from Sensor for each class with the sensorValueMappings list in it in the spec. Those classes would be like the example I showed above. They would also replace the current version of the TouchSensor class which I implemented by hand.

[ddemidov]

Ah, it all makes sense to me now (should not do PR reviews so early in the morning)! Yes, we have single Sensor class with all the generic properties, and specialized classes that are basically empty now. We would just extend those. Sorry for not understanding this earlier.

It looks fine to me then. Even creating special sensor type section makes sense.

[WasabiFan]

Sorry for not understanding this earlier.

It's fine -- I confess to not really understanding what I was saying either :wink:

Even creating special sensor type section makes sense.

OK, I will re-include that and add the other sensor types. I'm currently working on implementing this theoretical system for JS so that I can make sure that it has all the necessary information.

[WasabiFan]

@ddemidov I just went through and transferred all the sensors and added the new info. I also updated the JS binding in this branch to generate based off of this data, so we know that it has everything that we need.

I also found a few conspicuous docs issues along the way (which you probably saw I opened issues for) so there might be some slight changes to this based on the resolution of those. I tried to make my best judgement as to what was the proper text, so I expect we're fine.

[ddemidov]

Thanks, I'll try to apply the changes to cpp and python on the weekend.

Cheers, Denis On Nov 26, 2015 10:22, "Wasabi Fan" notifications@github.com wrote:

@ddemidov https://github.com/ddemidov I just went through and transferred all the sensors and added the new info. I also updated the JS binding in this branch to generate based off of this data, so we know that it has everything that we need.

I also found a few conspicuous docs issues along the way (which you probably saw I opened issues for) so there might be some slight changes to this based on the resolution of those. I tried to make my best judgement as to what was the proper text, so I expect we're fine.

— Reply to this email directly or view it on GitHub https://github.com/ev3dev/ev3dev-lang/pull/127#issuecomment-159834197.

[rhempel]

Folks, sorry I have not been following this thread. I'm working with @dlech to make a kind of pseudo-sensor for the EV3 buttons, and it will also handle things like gamepads and mice in a general way. I'm also working on a Lua binding, which is what prompted the need for a non-ioctl way to get button state :-) Hopefully the spec.json changes are not too drastic, but my review will have to wait for the weekend.

[WasabiFan]

I don't think they're particularly drastic; the main thing is that you'll be able to replace your implementations of the extra sensors (ultrasonic, touch, etc) with something that is completely autogenerated and includes helper properties such as isPressed. We can wait to merge this until you get a chance to look and evaluate the impact it'll have on you and provide feedback.

[WasabiFan]

@rhempel Did you get a chance to review these changes and make sure that they make sense? I want to be sure that I get another person to read over this and confirm that there aren't any obvious mistakes.

[rhempel]

No, not yet, sorry. Hopefully tonight?

[rhempel]

OK, I have looked at the changes, and I think they make sense - as I understand it the language binding MUST generate the core API for each sensor (getters/setters) and can optionally generate the convenient helper functions (like isPressed()) I guess if you think about it, the motor API could be extended the same way...that's for another day. I will merge this into ev3dev-lang, and test with the Python bindings. @ddemidov and @WasabiFan, have you got your bindings lined up with the changed property (port_name -> address)?

[rhempel]

Whew! (I think)

[WasabiFan]

have you got your bindings lined up with the changed property (port_name -> address)?

I do not believe that I have completely updated, but with recent architecture changes in my code it should be pretty easy. I will look at it soon.

There's also a related topic that I'd like to discuss, but I will open a new issue for that later today or tomorrow -- mostly planning stuff.

[rhempel]

Well, it turns out that I messed the merge up a bit - I'll be trying to fix it for the next little while - but I did get the changes to my ev3dev-lang-python made to support the new special-sensor changes. Then I'll get the port_name change done, which will break everyone else's language bindings

[WasabiFan]

Well, it turns out that I messed the merge up a bit

Are you referring to the merge markers in the autogen files? Yeah, I'd say that's a problem :wink:

If not, I'd like to point out that there are merge markers in the autogen files.