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

Implement record patches feature #2051

Merged
merged 12 commits into from
Oct 28, 2020
Merged

Implement record patches feature #2051

merged 12 commits into from
Oct 28, 2020

Conversation

brusherru
Copy link
Contributor

There is no issue. 🙈

However, this feature makes it possible to create C++ structs using types of input terminals and unpack node by adding a record marker on the patch.

How to use:

  • create a new patch (named foo for example)
  • place some inputs (generics and pulses are prohibited)
  • place output-self
  • place record marker node

It automatically creates a C++ implementation and the unpack-foo patch.

Also, some custom types might be easily replaced with this thing.

@brusherru brusherru self-assigned this Oct 2, 2020
Copy link
Member

@nkrkv nkrkv left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Massive 👍

The one thing I miss from this PR is an end-to-end test fixture that would show the real C++ code generated for records pack/unpack.

@brusherru
Copy link
Contributor Author

I've added abstract xod/json/to-json node and specializations to the primitive types in the same library. And made an auto-creation of specialization for the record. So records might be easily serialized to JSON.
Check it out, please.
@nkrkv @evgenykochetkov

@brusherru
Copy link
Contributor Author

@nkrkv @evgenykochetkov it's done to review!
Last changes:

  1. I fixed my own bug in the transpilation (last fixup)
  2. I made record fields named with pin labels instead of index.

Copy link
Member

@nkrkv nkrkv left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You haven’t updated a fixture workspace/record-pack-unpack/__fixtures__/arduino.cpp. It still defines the struct as:

    struct Type {
        static constexpr typeof_PORT _0 = constant_input_PORT;
        typeof_N _1;
        typeof_STR _2;
        typeof_AREC _3;
    };

It’s a problem (1). The CCI server gave no error because of this, it’s a problem (2).

packages/xod-arduino/platform/nodes/record.tpl.cpp Outdated Show resolved Hide resolved
@brusherru brusherru requested a review from nkrkv October 21, 2020 13:31
packages/xod-project/built-in-patches.xodball Outdated Show resolved Hide resolved
packages/xod-project/src/Patch.rei Outdated Show resolved Hide resolved
packages/xod-project/src/messages.js Outdated Show resolved Hide resolved
@brusherru brusherru force-pushed the feat-records branch 2 times, most recently from 42d1a40 to 0442f18 Compare October 21, 2020 14:46
@brusherru brusherru requested a review from nkrkv October 21, 2020 14:46
@brusherru brusherru force-pushed the feat-records branch 2 times, most recently from 206c62f to 672bab8 Compare October 26, 2020 10:31
@evgenykochetkov
Copy link
Contributor

Screen Shot 2020-10-26 at 16 55 47

IMHO, the error message seems a bit unclear. I think it should mention output-self node.

@evgenykochetkov
Copy link
Contributor

Screen Shot 2020-10-26 at 16 58 34

@evgenykochetkov
Copy link
Contributor

Screen Shot 2020-10-26 at 17 13 49

/ws/sketchbook/be1830b1f421f4c8fa703a23a859de0cc02ba339c8743ce339975ba147933f47/sketch/sketch.ino:1487:72: error: no member named 'field_IN' in 'xod::____bar::Node<'\x0D'>::Type'
        static constexpr typeof_OUT2 constant_output_OUT2 = typeof_IN::field_IN;
                                                            ~~~~~~~~~~~^
/ws/sketchbook/be1830b1f421f4c8fa703a23a859de0cc02ba339c8743ce339975ba147933f47/sketch/sketch.ino:2002:17: note: in instantiation of template class 'xod::____unpack_bar::Node<xod::____bar::Node<'\x0D'>::Type>' requested here
Node_9 node_9 = Node_9({ /* @/foo */ });
                ^
/ws/sketchbook/be1830b1f421f4c8fa703a23a859de0cc02ba339c8743ce339975ba147933f47/sketch/sketch.ino:2014:19: error: cannot convert 'xod::XString' (aka 'List<char>') to 'xod::Node_13' (aka 'int') without a conversion operator
Node_13 node_13 = Node_13(XString());
                  ^~~~~~~~~~~~~~~~~
/ws/sketchbook/be1830b1f421f4c8fa703a23a859de0cc02ba339c8743ce339975ba147933f47/sketch/sketch.ino:2317:13: error: 'xod::Node_13' (aka 'int') is not a class, namespace, or enumeration
            Node_13::ContextObject ctxObj;
            ^
/ws/sketchbook/be1830b1f421f4c8fa703a23a859de0cc02ba339c8743ce339975ba147933f47/sketch/sketch.ino:2324:20: error: member reference base type 'xod::Node_13' (aka 'int') is not a structure or union
            node_13.evaluate(&ctxObj);
            ~~~~~~~^~~~~~~~~
/ws/sketchbook/be1830b1f421f4c8fa703a23a859de0cc02ba339c8743ce339975ba147933f47/sketch/sketch.ino:2414:39: error: member reference base type 'xod::Node_13' (aka 'int') is not a structure or union
            ctxObj._input_IN = node_13._output_OUT;
                               ~~~~~~~^~~~~~~~~~~~
5 errors generated.

@brusherru
Copy link
Contributor Author

@evgenykochetkov just pushed two fixups with a tweaked error message and fix for constants.

Copy link
Contributor

@evgenykochetkov evgenykochetkov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@brusherru brusherru merged commit ca24195 into master Oct 28, 2020
@brusherru brusherru deleted the feat-records branch October 28, 2020 15:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants