Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Could we utilize the attribute documentation from the device driver repo? #51

Open
WasabiFan opened this issue Apr 17, 2015 · 7 comments

Comments

@WasabiFan
Copy link
Member

Currently, the ev3dev.org docs are generated based on data placed in the source files of the ev3dev drivers in the lego-linux-drivers repo. The comments in the source files are translated into the documentation pages via @dlech's Python script.

I was thinking that this is similar in concept to what we are doing with our own JSON spec system, only we manually update our spec based on those docs changes. I was wondering what the technical limitations would be for us to generate our spec off of the same source (the comments in those source files). Basically, my query comes down to this:

  • Is the data in those source files structured, or is it just a markdown list?
  • Is all the data that our system needs maintained in that source already (property name, type, accessibility)?

My hope would be that, if I could get my script to pull from that data, we wouldn't be left with nearly as many little discrepancies between the kernel functionality and what I put into the spec.

@dlech
Copy link
Member

dlech commented Apr 17, 2015

There is a json file for sensor definitions, so you could automatically generate classes for each specific type of sensor.

For the various subsystems though, only half of what you need is available. You could easily parse the source code and get the attribute names and read/write access. However, everything in sysfs is a string, so you wouldn't know if it was actually an integer or an array (spaces separated string).

@WasabiFan
Copy link
Member Author

OK, thanks for the info. I'll take a look at those files and see what I might be able to do.

@ddemidov
Copy link
Member

ddemidov commented May 4, 2015

It would also be great to pull the documentation part of the comments (the one that is being used for the site generation) into the autogen/spec.json. It could then be used either as comments in the generated sources (which, at least in case of C++, could be used with a documentation tool like Doxygen), or as python docstrings. The latter would provide an interactive help system for python users.

@WasabiFan
Copy link
Member Author

I've been looking into this, and at the moment I don't think that we would be able to make this work. This stems from three main problems:

  • Insufficient information. Some of the information that we need ( such as specific return types) aren't easily accessible in the current comment structure.
  • Unstructured data. A fair amount of the data is in unstructured comments, which makes it non-trivial to parse (and prone to errors).
  • Multiple disparate systems. As I understand it, there are currently two systems for this information: comments in the code itself and a separate JSON file (for the sensors).

So, I don't think that this could work directly. However, if @dlech is open to re-structuring the info in the drivers so that it includes all of this information in an accessible way (either by moving it all into structured comments or all into external data files), I think that we would be able to make this work. The main benefits that I see (across the ev3dev project) are:

  • No ambiguity. There is one place that this data is defined, and everything else stems from that.
  • User-friendly documentation generation. Having all of this data means that we can provide richer documentation experiences to users on ev3dev.org. I'm thinking about things like full search of available properties and/or an interactive tree of device classes and info.
  • Easily accessible for projects like ev3dev-lang. I'm sure that there are other applications for this data, but it would be a great help for this project so that we can fully support the range of languages that can be run on the EV3.

So, @dlech, what are your thoughts? Would moving the motors over to the JSON system with the sensors give us the kind of data we are hoping for?

@dlech
Copy link
Member

dlech commented May 4, 2015

There is an existing JSON file for motors, but as I think I have mentioned before, it is currently maintained by hand. And also, like the sensors, this is just information about the specific motor models, not about the *-motor classes. So, I don't think that helps much.

Also, in the case of motors, rather than directly mapping all of the attributes, I think you could make some more user-friendly functions. Many of the attributes only apply to a certain run command and don't take effect until the run command is sent. So, for example, instead of having a public command property and a public time_sp property, you have a public run_timed function with time_sp as an argument. Internally, the function writes time_sp to the corresponding attribute and the writes run-timed to the command attribute.

Back to the original issue though, If either of you want to devise a structured comment format and implement a parser to generate the website documentation, go for it. It would be a welcome improvement, but it's not something that I am going to do myself. Right now, the unstructured stuff is just markdown so it goes straight to the website (with some slight changes).

If you were to do this, I would suggest adapting the port definition parser to a new "sysfs attribute parser". You could look for function names that end in _show and _store (and _read and _write for binary attributes). Then, you would have to make a template for the website that reads the generated data. And, of course, the existing markdown documentation comments would have to be split up into individual attribute comments and the missing information filled in.

Or, you could take the gamble that we are done making major changes to the kernel drivers (and hopefully we are - I don't have any more big plans for this at the moment) and just manually copy the information once and maintain it by hand.

@WasabiFan
Copy link
Member Author

Also, in the case of motors, rather than directly mapping all of the attributes, I think you could make some more user-friendly functions.

I've had the same thought (I even have a test implementation of something like this here). This is another one of those things that I had hoped to do a long time ago, but just haven't been able to do because development on these bindings hasn't been able to keep up with kernel changes (until now, hopefully). The big issue that I have had with this that has kept me from perusing a full implementation of these helpers is the wide variety of edge cases that can arise. For example, in a run_timed method, there are many properties that can impact the operation of the motors that you need to reset to make sure that you can produce repeatable results (regardless what the previous command did). This issue has been mostly resolved with the recent motor driver overhaul, so I think now is the best time to start on this.

Back to the original issue though, If either of you want to devise a structured comment format and implement a parser to generate the website documentation, go for it.

I think I'll take a shot at this if I get the chance. This will be a fairly large set of changes (and I'll have to recall my Python knowledge) but I think I can probably figure this out if I get a few days when I'm not too busy.

Or, you could take the gamble that we are done making major changes to the kernel drivers (and hopefully we are - I don't have any more big plans for this at the moment) and just manually copy the information once and maintain it by hand.

Sounds good. Assuming there aren't any major overhauls on the horizon it shouldn't be too hard to keep up.

As a general takeaway, I think I'm going to push to officially release the current version of our language bindings in this repo within the next week or two. @ddemidov has been releasing his python binding much more frequently than I have been able to with the rest of the repo as a whole, and I'd like to pick up our pace to fit more in line with his.

@ddemidov
Copy link
Member

ddemidov commented May 5, 2015

Also, in the case of motors, rather than directly mapping all of the attributes, I think you could make some more user-friendly functions.

I've had the same thought (I even have a test implementation of something like this here).

I initially had something like this for python bindings, but I think that was imperfect (for the same reason @WasabiFan mentioned: too many possible options). I prefer the new ev3dev API (with command property). And the latest change to python bindings makes the following possible:

(m.speed_sp, m.position_sp, m.command) = (900, 360, 'run-to-rel-pos')

I think this is both concise and flexible.

As a general takeaway, I think I'm going to push to officially release the current version of our language bindings in this repo within the next week or two. @ddemidov has been releasing his python binding much more frequently than I have been able to.

I am sorry for spamming you with PRs :).

Since you mentioned stabilization, I'd like to discuss one more major(ish) change to C++/python API. I'll open a new issue for it (#67).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants