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

python library can't handle bare Tuple<T,S> return type from Arduino Library #23

Closed
gpantazis opened this issue Nov 9, 2021 · 8 comments

Comments

@gpantazis
Copy link

Describe the bug
When an Arduino function with return type Tuple<T,S> (e.g Tuple< int, const char*>) They Python library fails with ValueError: top level type can not be tuple. If instead an Object<T,S> is returned the system works fine.

This may be purely a documentation issue (or even PEBCAK) rather than a "bug" since I can't seem to find anywhere where it mentions Tuples cant be returned "bare". The only hint I can find is here and here where it is mentioned Objects<> preserve internal structure.

NOTE: This is with Python3.9 on a Windows system. No Linux system handy to test right now but I am 99% certain that it is not an OS level issue.

The Traceback is:

File "test_tuple_rpc.py", line 5, in <module>
    a = ...\simple_rpc.SerialInterface("COM12")
  File "simple_rpc\simple_rpc.py", line 58, in __init__     
    self.open(load)
  File "...\simple_rpc\simple_rpc.py", line 208, in open        
    super().open(handle)
  File "...\simple_rpc\simple_rpc.py", line 149, in open
    self._get_methods()
  File "...\simple_rpc\simple_rpc.py", line 123, in _get_methods
    method = parse_line(index, line)
  File "...\simple_rpc\protocol.py", line 124, in parse_line
    method = _parse_signature(index, signature)
  File "...\simple_rpc\protocol.py", line 69, in _parse_signature
    method['return']['fmt'] = _parse_type(fmt)
  File "...\simple_rpc\protocol.py", line 31, in _parse_type
    raise ValueError('top level type can not be tuple')
ValueError: top level type can not be tuple

To Reproduce
Steps to reproduce the behaviour:

Load test_tuple_rpc.ino on Arduino:

#include "simpleRPC.h"

void setup() {
    Serial.begin(9600);
    while(!Serial) {}
}

//Object<int, const char*> test_func()
//{
//    Object<int, const char*> t ={0, "OFF"};
//    return t;
//}

Tuple<int, char> test_func()
{
    Tuple<int, char> t ={0, "O"};
    return t;
}

void loop() {
    interface(Serial, test_func, "test_func:");
}

Run test_tuple_rpc.py on desktop machine:


a = simple_rpc.SerialInterface("COM12")
b = a.test_func()
print(b)
a.close()

Expected behavior
If it is expected that Tuples can't be top level return types on their own then perhaps that needs to be explicitly mentioned in the docs.

If Tuples are expected to be top level return types then perhaps the python code could "cast" them to Object on the python side? That second option sounds like it would break stuff. Alternatively Tuple and Object could be merged to a single type (if so I personally prefer the Tuple name to match the C++ equivalent of Vector)

@jfjlaros
Copy link
Owner

The internal structure of both Tuple and Object types are nested, e.g., Tuple<char, int, float> is shorthand for Tuple<char, Tuple<int, Tuple<float, Tuple<>>>>. If these types were merged, data in such a structure would be presented to the caller as an overly complicated nested structure, e.g., ('a', (10, (1.2))) instead of the expected ('a', 10, 1.2).

To circumvent this, we need two types of these objects, one that serialises to a flat list and one that has an explicit internal structure. To stick with the previous example, Object<char, int, float> is shorthand for Object<Tuple<char, Tuple<int, Tuple <float, Tuple <>>>> and data in such a structure is presented to the caller as a simple tuple, e.g., ('a', 10, 1.2).

So in a sense, the Object type places the parentheses around the values. This however means a bare Tuple no longer has a representation for the caller.

I will try to explain this in the documentation as well.

@jfjlaros
Copy link
Owner

I tried to explain it in this note.

@gpantazis
Copy link
Author

Yeah after playing with it more I see the concept and understand the idea. Indeed merging them makes little sense. Perhaps my confusion is purely based on semantics about what the flat type is called and what the nested type is called. In a sense I was thinking of Object as the flat one, perhaps due thinking about them like structs in C, and Tuple as the nested one.

Regardless this was my mistake and I agree on all points you have made. I obviously missed the note! Sorry for the noise. Feel free to close the Bug Report. As always thank you for the excellent library

P.S
As an aside, I personally prefer to use the STL, even on Arduino, so I have patched your C++ library to at least support std::string, but I could likely include support for std::vector and std::array (or others). Is that something you are interested in? If so I can open a feature request on the other repo and eventually put in a PR for them. I don't mind maintaining internal patches for the stuff we are working on but I only bring it up because support for Arduino String is there. I understand what we are doing is somewhat niche (power management for an RF system) so I don't want to overstep.

@jfjlaros
Copy link
Owner

You did not miss the note, I wrote it after you opened this issue.

I would have preferred using the STL as well, but to my knowledge it is not distributed by default with the Arduino cores. If this is the case, it may be a bit more difficult for an end user to use this library. Of course I could be wrong here.

@jfjlaros
Copy link
Owner

I just thought I could give it a go, but the standard installation does not seem to work, at least not in my setup. As soon as I include ArduinoSTL.h, I get the following errors.

.../libraries/ArduinoSTL/src/del_opvnt.cpp:25:58: error: 'nothrow_t' in namespace 'std' does not name a type
 _UCXXEXPORT void operator delete[](void* ptr, const std::nothrow_t& ) throw(){
                                                          ^~~~~~~~~
.../libraries/ArduinoSTL/src/del_ops.cpp:25:50: error: 'std::size_t' has not been declared
 _UCXXEXPORT void operator delete(void* ptr, std::size_t) throw(){
                                                  ^~~~~~
.../libraries/ArduinoSTL/src/del_opnt.cpp:25:56: error: 'nothrow_t' in namespace 'std' does not name a type
 _UCXXEXPORT void operator delete(void* ptr, const std::nothrow_t& ) throw() {
                                                        ^~~~~~~~~
.../libraries/ArduinoSTL/src/del_opvs.cpp:25:53: error: 'std::size_t' has not been declared
 _UCXXEXPORT void operator delete[](void * ptr, std::size_t) throw(){

@gpantazis
Copy link
Author

gpantazis commented Nov 15, 2021

Mini rant to start: I honestly hate how Arduino pushes this weird C within C++ style of programming. I understand they are trying to be beginner friendly but I feel it creates a bunch of weird corner cases when you are trying to do something a little more involved. Mini rant over.

So honestly I have not used ArduinoSTL.h but those errors seemed strange. A quick Google search points to these threads: 1 and 2 which tl;dr suggest changing #include <ArduinoSTL.h> to #include "ArduinoSTL.h" or changing Arduino IDE version

All that said the way I use the STL is to include -lstdc++_nano in the build flags. For the Teensy line of boards I modify boards.txt > teensyXX.build.flags.libs to include the additional library. For Arduino it is somewhat more involved, requiring either arduino-cli or modifying other .txt. files (some info here). The nice thing about lstdc++_nano is it does not throw, is relatively light weight and imo fulfills the role of ArduinoSTL.h.

Of course modifying Arduino files is daunting for many users so I understand that any STL inclusion would cater to a niche of a niche.

(edit: made links visible)

@jfjlaros
Copy link
Owner

Changing the <> to "" did not help. I am also not sure whether this library is still maintained.

I mainly use arduino-cli for my projects and adding a build flag seems to be rather straightforward. Do you perhaps have an example project where std::string is used together with build options, include and library paths?

I would not mind adding some optional extra complexity. We could think about an additional include for STL support in conjunction with some build instructions.

@jfjlaros
Copy link
Owner

I opened discussion #24 for this topic.

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

2 participants