jfjlaros / simpleRPC

Simple RPC implementation for Arduino.
MIT License
50 stars 17 forks source link

Support for SAMD architecture #8

Closed chrisflesher closed 3 years ago

chrisflesher commented 3 years ago

I'm using a feather M0 and would like to use this library for SAMD architecture but when I try to compile it's saying the library is only for AVR micro-controllers.

I'm imagining a SerialIO class would replace HardwareSerialIO and SoftwareSerialIO, like this:

#ifndef SIMPLE_RPC_SERIAL_IO_H_
#define SIMPLE_RPC_SERIAL_IO_H_

#include <Arduino.h>

class SerialIO {
  public:
    SerialIO(void) {}
    void begin(Stream&);
    size_t available(void);
    size_t read(byte*, size_t);
    size_t write(byte*, size_t);
  private:
    Stream* _ss;
};

typedef SerialIO HardwareSerialIO;  // for backwards compatibility
typedef SerialIO SoftwareSerialIO;  // for backwards compatibility

#endif
chrisflesher commented 3 years ago

I tried making this change and performed the following steps:

Now am getting the following error: simple_rpc: error: top level type can not be tuple

jfjlaros commented 3 years ago

Perhaps a separate plugin and guards around the serial plugin includes in simpleRPC.h would be a better approach.

chrisflesher commented 3 years ago

That would work also. I tried reducing the demo program to the following and it worked! So maybe there is a separate issue for tuples?

#include <simpleRPC.h>

HardwareSerialIO io;

int inc(int a) {
  return a + 1;
}

void setup(void) {
  Serial.begin(9600);
  io.begin(Serial);
}

void loop(void) {
  interface(
    io,
    inc, F("inc: Increment a value. @a: Value. @return: a + 1.")
  );
}
chrisflesher commented 3 years ago

I think HardwareSerial and SoftwareSerial inherit from Stream? See this ref: https://arduino.stackexchange.com/questions/54955/how-to-write-a-library-that-support-both-hw-and-sw-serial-communication-and-allo

This might also be a different way to solve #7.

jfjlaros commented 3 years ago

I am not sure about this tuple error. It does not occur when using the HardwareSerial plugin.

jfjlaros commented 3 years ago

The philosophy behind these plugins is to only pass a pointer to a communication interface. This way, the plugin does not need to know about any of the configuration possibilities that the interface may have. Making a more generic interface for both hardware and software serial interfaces would mean more maintenance, that is why I would prefer a separate thin wrapper.

chrisflesher commented 3 years ago

Sounds good.

jfjlaros commented 3 years ago

I am open for pull requests, if you have something working.

chrisflesher commented 3 years ago

I can't seem to push to this repo? Do I need to fork it and then push to that?

chrisflesher commented 3 years ago

Ok, I think I figured it out...

chrisflesher commented 3 years ago

I submitted a PR, I tried to use #defines as suggested but found Arduino was still trying to compile the HardwareSerial and SoftwareSerial io.cpp files even though the header files were not being used. I could add #defines to those files also but seemed simpler to inherit. I know this isn't what you requested but maybe will still work? Do you have an AVR you're able to test this with easily to see if you can still configure SoftwareSerial or HardwareSerial like you want?

It seems like the other error I was getting about tuples was because I was using python 2.7 instead of python 3.

chrisflesher commented 3 years ago

This library is really awesome btw!!

jfjlaros commented 3 years ago

Thank you. I will perform some tests tomorrow.

jfjlaros commented 3 years ago

Everything seems to check out fine. I just merged your changes.

chrisflesher commented 3 years ago

Cool, thanks!

jfjlaros commented 3 years ago

I see that both EthernetClient as well als TwoWire ultimately inherit from Stream. Perhaps this fix makes all the plugins redundant.