ev3dev / ev3dev-lang

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

C# API again #129

Open pgrudzien12 opened 8 years ago

pgrudzien12 commented 8 years ago

Currently implements:

Whenever possible constants, basic methods and comments are generated with liquid.

I had to change camel_case as 7e098d241e19445eb076e636e2d29846889f6f1b changed behavior for undefined values.

WasabiFan commented 8 years ago

I had to change camel_case as 7e098d2 changed behavior for undefined values.

That's great :wink: -- sorry I changed it without making sure it behaved as before! Thanks for taking the time to fix my mistakes.

Currently implements: ...

It looks good to me. You'll probably want to implement those others at some point, but it's a lot to deal with at once and we still haven't even gotten all of our current code caught up with the new classes so it's completely understandable.

Whenever possible constants, basic methods and comments are generated with liquid.

Overall, how'd that go? I would hope that it made it easier for you to get started, but I know that we don't have much in the way of "getting started" docs or info on how to configure autogen; we definitely don't want it to feel like a chore. What do you think we need to improve to make it as easy as possible for new people to develop bindings? And were the templates tiresome to create (figuring out errors, formatting whitespace, etc.) or was it manageable? Additionally, given that you had to hunt down that autogen bug, how easy was it to find? I'm just looking for honest feedback, so feel free to be blunt.

A few notes:

I'm going to give this a day to make sure that @ddemidov gets a chance to look at it, but after that I'll merge it. Let me know when you're done making any changes that you want to make.

ddemidov commented 8 years ago

I have little to no experience with C#, so I can not comment on the code :). In addition to @WasabiFan comments, it would be nice to have a basic example (or two) of using the library. The examples may be based e.g. on Explor3r robot by Laurens Valk (Laurens was kind enough to explicitly allow using the robot instructions for this purpose). Having a working example would provide an immediate overview of the library possibilities.

pgrudzien12 commented 8 years ago

Hey thanks for your interest. It's really nice to see that people are so engaged as you are in their side projects.

@WasabiFan I'll give you my feedback in a few days - I just don't have much free time besides weekends.

@ddemidov I agree that it would be good to have example. I will see into that after improvements to library itself.

pgrudzien12 commented 8 years ago

@WasabiFan

You'll probably want to implement those others [features] at some point

You are right. I'll try to do as much as possible.

Overall, how'd that go?

It was okish. Readme.md file under autogen helped a lot at the beginning. After this initial learning period I was able to use cpp lang and others as examples and I have to say that this part was pretty smooth.

The worst part I think was liquid syntax itself. After it goes beyond 10-15 lines of template it becomes unmanageable and needs rethinking. It would be helpful to have a feature in liquid like calling generation of part from within working generator. Ending lines with %}{% felt very uncomfortable and clutters template a lot. I think that it's not well fitted into this kind of generating code. It is ok for html or places where amount of code exceeds template code but I ended up with exactly opposite ratio. However I don't know any template engines that would meet your needs.

I think that having spec.json file is a great idea. I felt that some of the parts that are not currently autogenerated could be moved into this file as well eg. some constants like in1, out1.

Additionally, given that you had to hunt down that autogen bug, how easy was it to find?

Finding it wasn't a problem. It made me think about my code if I should detect undefined values beforehand. Sadly I don't know how to handle it in liquid.

It would be great if you added C# to the list of languages on repo README

I will. Hopefully today.

... brief description of the library, information on compilation/usage ...

Yes I feel that without it even fairly determined folks wouldn't like to use it. It's a must.

just add a submodule that references that repo

Hell I don't know yet how to do it but it seems reasonable. I'll look into this soon. If it require you to reject PR then don't hesitate. I'll make another if needed after catching up with this idea.

WasabiFan commented 8 years ago

The worst part I think was liquid syntax itself.

I definitely agree and feel your pain -- liquid isn't very good at generating code in which you're trying to manage whitespace. I had been looking for alternate templating engines, but there really aren't any that do what we need as far as I can tell. And obviously, transitioning the templates that we have now would be a pain -- but it might be worth it if we could find a good alternate system.

Sadly I don't know how to handle it in liquid.

That's why I added the extra methods in that definition file for autogen -- my hope is that you don't have to implement any complex logic in liquid. Feel free to add autogen functions whenever there is more involved functionality that you need.

If it require you to reject PR then don't hesitate.

Definitely not! I have no problem with you including the files directly, and if you haven't used submodules I would recommend against them at the moment. They are pretty complex and don't provide much benefit for most things. So just make the changes that you want to on this PR and we'll merge it as-is.

ddemidov commented 8 years ago

I had been looking for alternate templating engines, but there really aren't any that do what we need as far as I can tell.

Have you been looking outside of javascript? We could use same spec.json with some other scripting language, like python. I don't have much experience with text templates, but quick search came out with e.g. http://www.cheetahtemplate.org/, and I am sure there are others.

rhempel commented 8 years ago

The nice thing about a templating engine is that we can replace it :-) As ling as we're based on spec.json we're all good.

pgrudzien12 commented 8 years ago

Maybe mustache? https://github.com/janl/mustache.js. It is widely used.

WasabiFan commented 8 years ago

We should open a new issue if we want to discuss the templating language.

pgrudzien12 commented 8 years ago

It took me a week but i finally made it:

C# code is still a little slow on startup but I'm looking forward for RTM version of DNX/dotnet that will add native compilation feature.

Let me know if this is enough for you to merge it.

I'd like to upload packed version of ev3dev up to nuget.org to ease development. What do you think about that? I'd call it simply ev3dev if you don't mind. If you would suggest better name for it just let me know.

rhempel commented 8 years ago

I think that we may need to be a little more protective of ev3dev as a standalone word or package name. In my mind ev3dev is the term we are using for the entire distribution - and it's a little wierd running ev3dev on other platforms, but the intent is to support LEGO compatible shields on those platforms. So I'd prefer if you named your binding ev3dev-csharp or something like that - whatever you like - but please not just ev3dev by itself to avoid confusion. Anyone else have any comments for or against?

dlech commented 8 years ago

I'd call it simply ev3dev if you don't mind\

If you are writing a library that adheres to the ev3dev-lang spec, you should use the namespace (and package name) Ev3devLang. Also see the discussion in #133.

Also, as a potential contributor to the C# language bindings, I would prefer that you maintained the library as a submodule rather than in the ev3dev-lang repository directly. This way, I can just clone the ev3dev-lang-charp repository (or whatever you choose to call it) instead of having to clone all of ev3dev-lang.

rhempel commented 8 years ago

Yup, that's the way we've been doing ev3dev-lang-python, and how I plan to finish of ev3dev-lang-lua

pgrudzien12 commented 8 years ago

Ok I see your concerns regarding ev3dev package/namespaces. There's no reason to worry about that. I'll do quick rename to avoid any confusion and misunderstandings.

@dlech Also if you prefer to have it as a submodule then I'll look into that and see what I can do.

dlech commented 8 years ago

There are some tips on submodules in #30.

rhempel commented 8 years ago

I am going to hold off on merging this until we (@dlech) get the next image out the door.

pgrudzien12 commented 8 years ago

As requested I've replaced code with submodule that will be maintained at https://github.com/pgrudzien12/ev3dev-lang-csharp. This repo contains all the csharp specific code.

I've also renamed namespace to meet your needs.

If there is anything I can do let me know.

WasabiFan commented 8 years ago

@pgrudzien12 We recently changed the structure of our repo to make it so that, aside from adding your language to the list of language comment formats, you don't have to modify the main repo to be able to add and autogen your code. See #142 for more information.

This means that you don't need to deal with your code inside a submodule after all -- sorry for the extra work and long wait!