# Need Critique of Automatic Neopixel library

**URL:** <https://forum.xod.io/t/need-critique-of-automatic-neopixel-library/1567>\
**Category:** Uncategorized\
**Created:** [December 16, 2018, 9:04pm UTC](https://forum.xod.io/t/need-critique-of-automatic-neopixel-library/1567 "2018-12-16T21:04:36Z")\
**Posts on this page:** 2\
**Page:** 1

<div class="post-metadata">

**Author:** ![awgrover](https://avatars.discourse-cdn.com/v4/letter/a/74df32/32.png) [@awgrover](https://forum.xod.io/u/awgrover)\
**Post date:** [December 16, 2018, 9:04pm UTC](https://forum.xod.io/t/need-critique-of-automatic-neopixel-library/1567/1 "2018-12-16T21:04:36Z")

</div>

I’d like a critique of my automatic conversion of the Adafruit NeoPixel library. It’s a proof-of-concept, so what should be done differently/better?

It’s [published](https://xod.io/libs/awgrover/adafruitneopixel/), so “Add Library…”, or examine [code](https://github.com/awgrover/xod_lib/tree/master/lib/awgrover/adafruit-neopixel-ll).

Keep in mind that this used an automatic process, to produce a minimally useful library for XOD.

My goals:

- The naming should reflect the orginal Arduino IDE library, so people can find it, and know what it is.
- Do a directy translation of the class and it’s methods as patches.
- Usable, but may need some polishing.

I try to credit the original library. See [discussion](https://forum.xod.io/t/converted-arduino-libraries-naming-and-owning/1525/4).

I tried to name the library close to the original, though I made a mistake with this one. It was supposed to be `awgrover/adafruit-neopixel-ll`, but shows up on the library list as `awgrover/adafruiteneopixel`. Would that have been a good name?

The name is supposed to reflect that it is a low-level translation from some other library. I couldn’t think of anything better than “ll” for low-level. Is there a better signal?

I assumed that this library would get fixed, wrapped, etc., to make it friendlier for XOD.

I assumed the library was a single class, so I produced patches for the constructors and each method. A direct conversion seemed the most obvious way to get something useful.

I had to change the spelling of the methods, from intercapped to dashed.

Some other libraries use a `x-device` name for the constructors. I used the class name, which is almost the same as the library name. Does that make it easier to find the constructor? Or confusing by not following the conventions?

From another [discussion](https://forum.xod.io/t/objects-xod-chaining-best-practices/1524/6) it seemed like the patches should be controlled by pulses. They cause state-change, so aren’t dataflow or functional.

The object is an input/output to every method, and I assumed that `buses` would get used a lot. Does that seem right?

There are no public instance-variables. I was going to try generating the equivalent of a getter & setter for them. Again, with pulses.

I did a simple mapping of _all_ numeric types to Number. I think this is not ideal, and may cause some problems eventually. You can see the `uint16_t`, etc. What do you think of this loss of type?

I wrote some examples, to test the library out a bit. That’s hand writing, not mechanical. Examples seem like a good idea?

It looks like `libclang's cindex` is not reporting all of the declarations to me. So, a few methods, and some arguments are missing. This is the sort of thing that would need to be fixed by hand. Cf. the static `Color`, the `fill()`, etc.

It seems necessary, sometimes, to change data to a pulse. For example, if a `counter` was updating a `set-pixel-color`, and choosing the pixel (like the `example-simple`). But, `set-pixel-color` only acts on a pulse. So, somehow, we have to generate a pulse. The only way I know to do that is with something like [awgrover/conversions/data-to-pulse](https://xod.io/libs/awgrover/conversions/data-to-pulse/). How should we bridge the functional/impure worlds?

The Neopixels need to know which order the RGB is in, which is handled by a list of values in the Arduino library (not even an enum!). I should use an `enum` for those when it becomes available. But, that also requires some manual intervention to get the values out of the Arduino library.

And, finally, look over the library. Should it have been done differently?

---

<div class="post-metadata">

**Author:** ![nkrkv](https://yyz1.discourse-cdn.com/flex027/user_avatar/forum.xod.io/nkrkv/32/8_2.png) [@nkrkv](https://forum.xod.io/u/nkrkv)\
**Post date:** [December 19, 2018, 3:55pm UTC](https://forum.xod.io/t/need-critique-of-automatic-neopixel-library/1567/2 "2018-12-19T15:55:28Z")

</div>

To put in summary: it is cool ❤

TL;DR

> [@awgrover](#):
>
> I try to credit the original library

Seeing the [library page](https://xod.io/libs/awgrover/adafruitneopixel/) I have no chance to overlook that Adafruit is tightly connected to the work.

> [@awgrover](#):
>
> Some other libraries use a `x-device` name for the constructors. I used the class name, which is almost the same as the library name. Does that make it easier to find the constructor? Or confusing by not following the conventions?

We use the simple name without the `-device` suffix for [quick start nodes](https://xod.io/docs/guide/wrapping-arduino-libraries/#quick-start-node) which are more accessible for newcomers and encapsulate the main function of a thing. Whereas a `xxx-device` is a dumb description which requires an action node (method) to do something useful. Used in advanced cases. It’s a pure convention but seems to work quite well.

> [@awgrover](#):
>
> The object is an input/output to every method, and I assumed that `buses` would get used a lot. Does that seem right?

This aspect requires more practical usage experience, more pain.

> [@awgrover](#):
>
> I did a simple mapping of _all_ numeric types to Number. I think this is not ideal, and may cause some problems eventually. You can see the `uint16_t` , etc. What do you think of this loss of type?

All values of `uint16_t` are directly representable with `Number`. There’s a performance penalty, but, well… we have to sacrifice something to make XOD possible.

However, `uint32_t` / `int32_t` is not directly representable with `Number` because values over 17M start to skip some integers. If it is important to a library, we have a problem. This case should be investigated.

> [@awgrover](#):
>
> Examples seem like a good idea?

They always are 🙂

> [@awgrover](#):
>
> It seems necessary, sometimes, to change data to a pulse. For example, if a `counter` was updating a `set-pixel-color` , and choosing the pixel (like the `example-simple` ). But, `set-pixel-color` only acts on a pulse. So, somehow, we have to generate a pulse. The only way I know to do that is with something like [awgrover/conversions/data-to-pulse](https://xod.io/libs/awgrover/conversions/data-to-pulse/). How should we bridge the functional/impure worlds?

IMO the pulse generation should be left up to a user. He could send the `trigger` pulse from `done` of an upstream node, from `pulse-on-change`, or whatever he wants. The implementation of such node should be done in a way which short-circuits the evaluation to quick skip if the pulse is not set:

```cpp
void evaluate(Context ctx) {
    if (!isInputDirty<input_trigger>(ctx)) {
        // The eval is provoked by a change of a functional value
        return;
    }

    // ...
    myDevice->myMethod(myArgs);
    emitValue<output_DONE>(ctx, 1);
}

```