ev3dev / ev3dev-lang

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

How to deal with platform-specific features? #109

Closed ddemidov closed 8 years ago

ddemidov commented 8 years ago

How do we deal with platform-specific features? For example, leds on ev3 have predefined names that we could put into specification to generate instances of Led class automatically. But we also want to support RPI, and leds there have different names.

See rhempel/ev3dev-lang-python#13 for the origin of this issue.

ddemidov commented 8 years ago

@rhempel suggested a json file per platform, but we could also have a section per platform inside the main spec.json. Then each of the language bindings could use the sections to, for example, generate separate file per platform. This way we would not need any changes to the current autogen system.

jabrena commented 8 years ago

It is not a bad idea.

rhempel commented 8 years ago

And then we don't need to change autogen either! Of course adding a platform section for some items will break the existing liquid templates, but there are not so many of them right now. I really enjoy the discussion here - we eventually get to a good solution by taking into account all ideas and selecting the best parts.

ddemidov commented 8 years ago

So something like this for ev3 leds?

diff --git a/autogen/spec.json b/autogen/spec.json
index ae1537c..e2442e8 100644
--- a/autogen/spec.json
+++ b/autogen/spec.json
@@ -701,14 +701,7 @@
                       "be specified via `delay_off` attribute in milliseconds."
                   ]
                 }
-            ],
-            "ev3LedColors": {
-                "red": { "red": 1.0, "green": 0.0 },
-                "green": { "red": 0.0, "green": 1.0 },
-                "amber": { "red": 1.0, "green": 1.0 },
-                "orange": { "red": 1.0, "green": 0.5 },
-                "yellow": { "red": 0.5, "green": 1.0 }
-            }
+            ]
         },
         "sensor": {
             "friendlyName": "Sensor",
@@ -1157,5 +1150,28 @@
                 }
             ]
         }
+    },
+    "platforms": {
+        "ev3": {
+            "led": {
+                "instances": [
+                    { "name" : "Red Left", "systemName": "ev3-left0:red:ev3dev" },
+                    { "name" : "Red Right", "systemName": "ev3-right0:red:ev3dev" },
+                    { "name" : "Green Left", "systemName": "ev3-left1:green:ev3dev" },
+                    { "name" : "Green Right", "systemName": "ev3-right1:green:ev3dev" }
+                ],
+                "groups" : [
+                    "name" : "Red", "entries" : ["Red Left", "Red Right"],
+                    "name" : "Green", "entries" : ["Green Left", "Green Right"],
+                ],
+                "colors": [
+                    { "name" : "Red", "values" : { "Red": 1.0, "Green": 0.0 },
+                    { "name" : "Green", "values" : { "Red": 0.0, "Green": 1.0 },
+                    { "name" : "Amber", "values" : { "Red": 1.0, "Green": 1.0 },
+                    { "name" : "Orange", "values" : { "Red": 1.0, "Green": 0.5 },
+                    { "name" : "Yellow", "values" : { "Red": 0.5, "Green": 1.0 }
+                ]
+            }
+        }
     }
 }
rhempel commented 8 years ago

I think that looks about right! We can add more entries to platforms as needed for different LED capabilities. Eventually this can be generalized to motor and sensor ports as well.

ddemidov commented 8 years ago

This will break js_led-color-methods.liquid (and same template for C++). @WasabiFan, are you ok with this?

WasabiFan commented 8 years ago

@ddemidov Yup; looks good. I'm fine with making the changes on my end. Thanks for the overall effort!

How do you plan to implement it? Are you going to have a runtime check to determine the platform, or generate multiple copies of certain files?

ddemidov commented 8 years ago

For C++ I was going to use preprocessor similar to this: https://github.com/ev3dev/ev3dev-lang/blob/fe797c147bda64a184bc709ae39a04b4fcd0226c/cpp/ev3dev.h#L60-L80

For python I was thinking about runtime check during class declaration. For example, this works:

p = 1

class A:
    if p == 1:
        def get(self):
            return 42
    else:
        def get(self):
            return 24

a = A()
a.get() # returns 42
ddemidov commented 8 years ago

@rhempel, I'll wait for you to merge pure-python branch into develop before pushing the specification changes.

rhempel commented 8 years ago

OK, while we are at it, @WasabiFan, is there a way to specify a particular "path" in spec.json? I am thinking of the helper command generator as an example. In a liquid template, if I want to find the list of commands, I need to iterate for propval in currentClass.propertyValues and look for a match like this: if propval.propertyName == "Command" Is there a way to write currentClass.propertyValues.[propertyName == "Command"] or something similar?

WasabiFan commented 8 years ago

I don't know of any such syntax; however, their docs are here, so you can look through them to see if there's anything that fits your needs. You can also edit the autogen script to add a custom filter that streamlines this type of access -- just look for the large object declaration with functions in it.

rhempel commented 8 years ago

I think I read in the docs that some packages provide a where filter - maybe we can get that added?

dlech commented 8 years ago

I think the where filter is specific to jekyll and not part of liquid.

rhempel commented 8 years ago

It's OK doing it the way we're doing it now - I mean, how often are we regnerating the bindings :-) I just hate to waste CPU cycles. In this case it's better than wasting programmer cycles implementing a way around this non-problem I have created. :-)