Markdown for plugin descriptions by FloEdelmann · Pull Request #1142 · OpenLightingProject/ola

FloEdelmann · 2016-10-23T15:46:53Z

I started progress to close #678 and project 1.

The plugin descriptions are converted to Markdown (README.md in each directory). I've also written a shell script to convert those Markdown files into C++ header files which can be called automatically during build. I don't know how to integrate that into the automake toolchain, so I leave this open for you ;)

You can tell me how to start and I'll try to do it myself.

peternewman · 2016-10-23T17:11:05Z

Here's an example of integrating a generated script into our Makefiles:
https://github.com/OpenLightingProject/ola/blob/master/include/ola/Makefile.mk#L46

peternewman · 2016-10-23T17:11:29Z

Thanks, BTW, this is a pleasant surprise. :)

peternewman

A few other bits that will need doing, some of which I'm sure you're aware of:

Switch the C++ code to use the new descriptions and remove the old ones.
Potentially reformat the plugin help so it fits within the 80 char line limit when it's in a string.
Add the new README.mds (and the script) to the relevant Makefile.mk's

peternewman · 2016-10-23T17:17:24Z

@@ -1,5 +1,37 @@
-USBDMX Plugin


Can you split the dev info out into a readme developer file of some sort please, as we don't need it to appear in the UI for the users.

It probably makes sense to standardise plugins/osc/README in the same way at the same time.

peternewman · 2016-10-23T17:19:18Z

+identifier="PLUGINS_${plugin}_DESCRIPTION_H_";
+identifier=`echo "$identifier" | tr '[:lower:]' '[:upper:]'`
+
+echo "#ifndef $identifier" > $outfile;


You'll need to add an appropriate header with licenses too. See https://github.com/OpenLightingProject/ola/blob/master/include/ola/make_plugin_id.sh for an example.

peternewman · 2016-10-23T17:23:33Z

+
+plugin=`basename "$path"`
+
+desc=`sed -e ':a' -e 'N' -e '$!ba' -e 's/\n/\\\\\\\\n"\n"/g' "$path/README.md"`;


That's an impressive number of backspace escapes!

Yeah, but it works 😀

peternewman · 2016-10-25T00:02:20Z

Given there are a number of plugins here, it probably makes sense to go through the review cycle with just one initially, then when we're happy you can apply the reviewed change style to all the other plugins, rather than repeatedly changing everything each time until it's sorted.

* call convert_README_to_header.sh from automake tools * use plugin_description in Karate plugin

FloEdelmann · 2016-10-25T18:48:20Z

See Karate plugin for how I did it. If that suits your needs, it's easy to add it for the other plugins, too.

* fix osc Makefile * fix line length * rename ifndef variable

FloEdelmann · 2016-10-25T19:58:33Z

Unfortunately, it doesn't work on Travis :(
I included the rule that matches all plugin description header files in plugins/Makefile.mk and that worked for me at home. Now Travis says it can't find a rule for target include/ola/plugins/karate_plugin_desc.h. Any idea why or how to make it better?

EDIT: The linter task has generated it. Now I'm completely confused.

peternewman · 2016-10-25T22:49:24Z

 include plugins/uartdmx/Makefile.mk
 endif
+
+include/ola/plugins/%_plugin_desc.h: plugins/%/README.md plugins/convert_README_to_header.sh plugins/Makefile.mk


I'd just stick it in the relevant plugin folder, so presumably plugins/%/%_plugin_desc.h, rather than putting it in include; we've got plenty of other headers in those folders already.

double %s don't work unfortunately. Should I rather go for plugins/%/PluginDescription.h?

peternewman · 2016-10-25T23:20:40Z

 endif

-EXTRA_DIST += plugins/karate/README.protocol
+EXTRA_DIST += plugins/karate/README.md \


We generally just go for += \ on the first line, then list everything indented beneath as you've done below.

peternewman · 2016-10-25T23:21:21Z

+BUILT_SOURCES += include/ola/plugins/karate_plugin_desc.h
 lib_LTLIBRARIES += plugins/karate/libolakarate.la
 plugins_karate_libolakarate_la_SOURCES = \
+	include/ola/plugins/karate_plugin_desc.h \


Can you fix the indenting please.

add other READMEs to fix Travis failing

FloEdelmann · 2016-10-26T08:44:44Z

Travis is still failing, but only on Mac OS / XCode. The compiler complains Expected ';' after top level declarator. It could be this issue: http://stackoverflow.com/questions/12160733/xcode-doesnt-recognize-the-word-class

So I'll try making it a .cpp file instead of a .h header.

This reverts commit 5106e0c.

peternewman · 2016-10-26T13:55:23Z

My suspicion is your sed command isn't working correctly on the Mac, see this line you've got '0.1\n"n"==='; where it should be multiple lines or something:
2190const char plugin_description[] = "KarateLight - Version 0.1\n"n"=========================\n"n"\n"n"The plugin creates devices with a single output port. Info on the\n"n"KarateLight Hardware can be found at http://karatelight.de\n"n"\n"n"Unfortunately the site is in German only, but the maintainer will respond to\n"n"emails in English.\n"n"\n"n"\n"n"## Config file: ola-karate.conf\n"n"\n"n"device = /dev/kldmx0 \n"n"The path to the KarateLight device. Multiple entries are supported.";

You could try changing travis-ci.sh to cat out the file you're having issues with in the after_failure section.

According to http://stackoverflow.com/a/16576291, \n is not recognized on Mac OS as a newline character, but \[actual newline] works. Let's try it out :)

This reverts commit 2a51b74.

peternewman

Mostly just minor nits. I personally don't like convert_README_to_header.sh as a filename, particularly the mixed case, but that's entirely irrational and down to me, and I can't think of a legitimate reason to change it!

Have you tested that the output still looks okay via ola_plugin_info and the old web UI?

peternewman · 2016-12-03T12:48:15Z

+| 1          | 15            | 0            | 496                 |
+| 1          | 15            | 15           | 511                 |
+
+That is `Port Address = (Net << 8) + (Subnet << 4) + (Universe % 4)`


This should be % 16 at the end.

peternewman · 2016-12-03T12:52:25Z

+* Sensor Device, with a number of sensors implemented
+* Network Device, with E1.37-2 PIDs
+
+The number of each device is configurable.


Changing number to quantity would probably make it more readable.

Or "The number of each type of device is configurable."

peternewman · 2016-12-03T12:54:14Z

+The number of input ports to create up to a max of 32.
+
+`ip = [a.b.c.d|<interface_name>]`  
+The ip address or interface name to bind to. If not specified it will use


peternewman · 2016-12-03T12:55:19Z

+## Config file: `ola-espnet.conf`
+
+`ip = [a.b.c.d|<interface_name>]`  
+The ip address or interface name to bind to. If not specified it will use


peternewman · 2016-12-03T12:56:49Z

 # The OLA artnet plugin
 lib_LTLIBRARIES += plugins/artnet/libolaartnet.la
 plugins_artnet_libolaartnet_la_SOURCES = \
+    plugins/artnet/ArtNetPluginDescription.h \


Can we put this below ArtNetPlugin.h please.

peternewman · 2016-12-03T13:02:04Z

@@ -1,3 +1,6 @@
+OSC Plugin Developer Information


Is the file Markdown, or just something that looks similar? If the former, can we give it a .md extension.

It was just something similar but I transformed it.

Top marks for effort!

peternewman · 2016-12-03T13:03:15Z

+Set the DSCP value for the packets. Range is 0-63.
+
+`ip = [a.b.c.d|<interface_name>]`  
+The ip address or interface name to bind to. If not specified it will use


peternewman · 2016-12-03T13:03:50Z

+## Config file: `ola-sandnet.conf`
+
+`ip = [a.b.c.d|<interface_name>]`  
+The ip address or interface name to bind to. If not specified it will use


peternewman · 2016-12-03T13:04:00Z

+## Config file: `ola-shownet.conf`
+
+`ip = [a.b.c.d|<interface_name>]`  
+The ip address or interface name to bind to. If not specified it will use


peternewman · 2016-12-03T13:04:58Z

 endif

+EXTRA_DIST += plugins/stageprofi/README.md
+


Can we remove the blank line please.

peternewman · 2016-12-03T13:12:00Z

@nomis52 can you review the changes. @daveol can you take a look at the JS changes please.

I'm tempted to say we merge this as is, when the outstanding nits are fixed. Then get @FloEdelmann to do the other plugins promptly afterwards and merge them in a second PR to keep the diff sizes down. As long as he checks that no-one has updated plugin descriptions in the meantime (which seems unlikely aside from #1147 , then we should be fine).

FloEdelmann · 2016-12-03T22:44:35Z

+open as you step through the explanations.
+
+Let's get to it. If you make it all the way through this doc and I ever meet
+you in person I'll buy you a beer!


Ok I already made it through the doc 😆

Heh, you need to talk to @nomis52 for that, and fly to the states! Or I'll buy you one if I see you in the UK.

Those are both a bit far away for me unfortunately...

Well if you ever end up over either way...

FloEdelmann · 2016-12-04T00:00:27Z

The output for the old web UI / ola_plugin_info actually also improved for my taste.

peternewman · 2016-12-04T01:23:35Z

Excellent. As long as they look readable don't have formatting quirks and the line breaks etc still work, that's fine.

Test

peternewman

LGTM

peternewman · 2016-12-29T16:42:41Z

@nomis52 I'm happy with this, do you want to give it a once over too please?

FloEdelmann · 2017-01-05T19:50:12Z

Still no update?

As soon as this is merged, I'll update the other plugins to let them also use their README files in the C++ code.

see fd8e586

nomis52

My only comment is that I would have called these CONFIG.md.

Other than that it looks fine.

peternewman · 2017-01-06T16:53:29Z

Thanks @nomis52 . I think the benefit of them being README.md is that it partially self-documents on GitHub, as you get the introduction to each plugin in it's plugin folder, like so:
https://github.com/FloEdelmann/ola/tree/82a753fec37362d61f6c340e87f67cdd1e248f9d/plugins/artnet

I'm going to force merge this, so you can get working on the other plugins @FloEdelmann , as the test failure is related to some Travis changes I made the other day rather than your code.

FloEdelmann added 2 commits October 21, 2016 19:47

move plugin to description to README.md

2c207fd

Added shell script to generate C++ header file

846b1b8

peternewman added this to the 0.11.0 milestone Oct 23, 2016

peternewman self-assigned this Oct 23, 2016

Merge branch 'master' into plugins-md

ddc9bf1

peternewman requested changes Oct 23, 2016

View reviewed changes

FloEdelmann and others added 4 commits October 24, 2016 15:37

Add license to generated header files

695f44c

Merge branch 'plugins-md' of github.com:FloEdelmann/ola into plugins-md

837c0e1

Merge branch 'master' into plugins-md

722dd32

Merge branch 'master' into plugins-md

209de59

FloEdelmann added 3 commits October 25, 2016 14:13

Merge branch 'plugins-md' of github.com:FloEdelmann/ola into plugins-md

3e38b88

split developer READMEs into separate files

162e13a

It works :)

c2546fa

* call convert_README_to_header.sh from automake tools * use plugin_description in Karate plugin

fix linter errors

5955133

* fix osc Makefile * fix line length * rename ifndef variable

try BUILT_SOURCES instead of built_sources

bc8667a

peternewman requested changes Oct 25, 2016

View reviewed changes

FloEdelmann added 2 commits October 26, 2016 08:25

fix indention, change path to plugins/%/PluginDescription.h,

d507c3f

add other READMEs to fix Travis failing

Add missing files to Makefiles

1844edc

FloEdelmann added 3 commits October 26, 2016 10:54

change PluginDescription.h to .cpp

5106e0c

Revert "change PluginDescription.h to .cpp"

db746ea

This reverts commit 5106e0c.

try to fix XCode again

ffa4ea4

FloEdelmann added 2 commits November 1, 2016 22:36

change \n escaping

2a51b74

According to http://stackoverflow.com/a/16576291, \n is not recognized on Mac OS as a newline character, but \[actual newline] works. Let's try it out :)

Revert "change \n escaping"

2aa045f

This reverts commit 2a51b74.

peternewman requested changes Dec 3, 2016

View reviewed changes

FloEdelmann added 3 commits December 3, 2016 23:40

fix SPaG

70422ab

change README.devevloper to markdown

31a263a

Merge branch 'plugins-md' of github.com:FloEdelmann/ola into plugins-md

73aa512

FloEdelmann commented Dec 3, 2016

View reviewed changes

update Makefile

58958c3

peternewman previously approved these changes Dec 6, 2016

View reviewed changes

Merge branch 'master' into plugins-md

d8d3054

peternewman removed their assignment Dec 7, 2016

Merge branch 'master' into plugins-md

5ae7d92

peternewman reviewed Dec 10, 2016

View reviewed changes

peternewman requested a review from nomis52 December 20, 2016 00:16

Merge branch 'master' into plugins-md

59d775d

peternewman and others added 2 commits December 29, 2016 23:37

Merge branch 'master' into plugins-md

e635fa7

Merge branch 'master' into plugins-md

503ed41

Add some more help regarding the ArtNet use_limited_broadcast option

82a753f

see fd8e586

nomis52 approved these changes Jan 6, 2017

View reviewed changes

peternewman merged commit 7d25bbd into OpenLightingProject:master Jan 6, 2017

FloEdelmann deleted the plugins-md branch January 6, 2017 17:17

FloEdelmann restored the plugins-md branch January 6, 2017 18:05

FloEdelmann mentioned this pull request Jan 6, 2017

migrate all plugin descriptions to Markdown #1185

Merged

FloEdelmann deleted the plugins-md branch January 6, 2017 18:08

peternewman mentioned this pull request Jul 12, 2026

Add 0.11 news #2060

Draft


		plugin=`basename "$path"`

		desc=`sed -e ':a' -e 'N' -e '$!ba' -e 's/\n/\\\\\\\\n"\n"/g' "$path/README.md"`;

Uh oh!

Conversation

FloEdelmann commented Oct 23, 2016

Uh oh!

peternewman commented Oct 23, 2016

Uh oh!

peternewman commented Oct 23, 2016

Uh oh!

peternewman left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

peternewman commented Oct 25, 2016

Uh oh!

FloEdelmann commented Oct 25, 2016

Uh oh!

FloEdelmann commented Oct 25, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

FloEdelmann commented Oct 26, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

peternewman commented Oct 26, 2016

Uh oh!

peternewman left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

peternewman left a comment •

edited

Loading

FloEdelmann commented Oct 25, 2016 •

edited

Loading

FloEdelmann commented Oct 26, 2016 •

edited

Loading

FloEdelmann commented Dec 4, 2016 •

edited

Loading