Make UDP methods

[feinstein]

I miss UDP functionality in the library, I am coding it myself right now but I am facing a little dilema.

Most TCP methods are the same as the UDP ones, all the same code for sending, closing etc.. Just the connection part is different, where one needs a "TCP" parameter when opening a connection with the server and the other a "UDP".

I think the correct thing to do is to rename the TCP methods to TCPIP or something similar and let then be generic and the user selects if it's supposed to be TCP or UDP. The problem with this is breaking legacy code compatibility.

So what do you think it's best:

1- To add new methods for UDP, being just a copy of the TCP ones (with the exception of the connect method).

2- Make generic methods that will handle both TCP and UDP and erase the current TCP methods, which can lead to problems in older codes.

3- Do option 2 but keep the old TCP methods, marked as legacy code, to prevent breaking old code functionality. personally think option 3 is the one to go.

[tdicola]

Oh nice, thanks for contributing and looking at the UDP methods. I like method 3 since that keeps backwards compatibility but also cleans up the code. What you could do is add some internal-only private methods that implement the generic TCP & UDP logic (but which take a parameter to specify if it's TCP or UDP). Then modify the existing TCP functions to just call that new internal method and pass in the appropriate TCP parameter. The new UDP functions could call the same internal methods and pass in the UDP parameter. That way there's just one method with the main logic, the private internal methods, while the existing TCP and new UDP-specific functions are a simpler public interface that library users can call.

If that makes sense feel free to take a crack at implementing it and sending a pull request. Thanks again for the help!

[feinstein]

I thought about doing it this way, but there will be a function call for every TCP or UDP access, since we are talking about embedded systems, I rather avoid unnecessary calls... Probably it won't be a great performance impact, and I can inline it and it won't change a thing (if the compiler accepts the inline), but still the SIM800 documents don't make much distinction between TCP and UDP, so I am on the opinion that the library should reflect this.

Could you reopen the issue? I am used to work on closing issues when the code is merged, fixing the issue, but if you guys don't work like this its just fine.

[feinstein]

FYI @ladyada @tdicola I sent you an email a couple days ago about this issue and my current findings.

[tdicola]

Ack sorry been a little busy and haven't had a chance to reply sooner. Yeah it's a great point about the trade-off of performance vs. clarify in the code. Most of our libraries are optimized more for maintainability and not super high performance so I think a little bit of overhead for the function call is OK here. Marking it as inline is a good point--that might be the best of both worlds. Feel free to submit a pull request--thanks again!

[feinstein]

@tdicola I understand, I have been busy myself. I have had updated the Fona code to work with UDP, but I can't make it work here so I won't be submitting a pull request.

Actually in the email I sent you I explain what's going on. I am making a complete different code for my FONA, and I am afraid to push this code to you since it's so different I am afraid I will break some current boards functionalities. It's a lot to explain in here, I made it more clear in my email, but if you want we can set up a quick skype conversation so I can explain you further (and quicker) my concerns about this.

[Dygear]

So I'm going to pickup the mantel for this and add UDP functionality (I need it for my use case). I'm going to push the TCP functions into a more generic socket function call, with the TCP calling into them, but should be marked a deprecated (I'll emit the message to console).

  // Socket Functions
  boolean getLocalIP(char *ipAddress); // AT+CIFSR
  boolean socketConnect(char *mode, char *server, uint16_t port);
  boolean socketClose(void);
  boolean socketConnected(void);
  boolean socketSend(char *packet, uint8_t len);
  uint16_t socketAvailable(void);
  uint16_t socketRead(uint8_t *buff, uint8_t len);
  unit8_t socketState(void); // AT+CIPSTATUS // Returns the Integer Component of <state>.

  // TCP Raw Connections
  // DEPRECATED
  boolean TCPconnect(char *server, uint16_t port);
  boolean TCPclose(void);
  boolean TCPconnected(void);
  boolean TCPsend(char *packet, uint8_t len);
  uint16_t TCPavailable(void);
  uint16_t TCPread(uint8_t *buff, uint8_t len);

SIM800 Series AT Command Manual V1.09

[feinstein]

I also needed it for my application 3 years ago so I made it already, I just don't recall if I pushed it to my repository or not, I think I did.

Take a look at my FONA repository and clone it, I think I probably submitted a PR and Adafruit didn't merge, or something. I had other modifications there as well, so it might not be full compatible with whatever there's in the repository right now.

[feinstein]

Take a look at PR #50