Members | Sign In
MOVI User Community Forum > MOVI Question & Answers
avatar

Pull request for MOVIArduinoAPI

posted May 25, 2016 18:06:13 by Dylan
Gerald,

I spoke to you offline about the possibility of adding the option to construct an instance of MOVI() using an existing HardwareSerial * (useful for Arduino Mega with all its extra serial pins). At the time you were hopeful that there was already an existing #define that could be flipped easily. Unfortunately there doesn't seem to be a very straightforward way to do this as the API stands at version 1.03 because the Software/Hardware serial toggle is based on ARDUINO_ARCH_AVR. The Arduino API defines that macro for the Mega so it's stuck with SoftwareSerial.

I made what I believe is a pretty elegant enhancement to the API to allow for constructing MOVI with a HardwareSerial pointer and created a pull request on github in hopes that you would like to incorporate it. Basically it involves:

- Changing MOVI(uint8_t debugonoff) from MOVI(bool debugonoff) to avoid constructor overload conflict. You mention in a comment that "Arduino's' C++ does not allow for constructor overloading!". This may have been the issue you were finding.

- Changing the MOVI class to store only the base class Stream * for mySerial.

- Moving the #ifdef...#elif... for the various platforms from the the class specifiction to the body of init(...) where you call mySerial.begin. This is the only Serial specific method that you use that is not virtual from Stream.

I also added a small enhancement to the ask() methods to not call say() when the parameter to ask() is the empty string. This boosts the reaction time for ask() considerably when MOVI doesn't need to say anything first.

I hope you like these changes.

Dylan.
page   1
6 replies
avatar
GeraldFriedland said May 26, 2016 02:09:26
I commented on one small thing directly in the code. Other than that: let me test this out on various boards and then it can go into the next library download package.

Thank you so much, Dylan!

Gerald
avatar
Dylan said May 26, 2016 18:19:03
Changes made as requested.

Hopefully you didn't start testing it yet because I enabled use of the HardwareSerial * constructor by all platforms (not just AVR) with a couple of additional small tweaks. It seemed unusual to have a HardwareSerial * constructor but have it only be valid for AVR locking other platforms to Serial1, so I wanted to fix that.

Dylan.
avatar
GeraldFriedland said May 26, 2016 19:01:30
Dylan,

Nope it's all cool. I just merged the pull request in and will label it 1.04 for now. But I want to test it on a couple boards before I release it as a download package.

Thank you again!
Gerald
avatar
GeraldFriedland said Jun 20, 2016 04:02:01
Dylan,

I finally had time to test your contributions. I changed one thing and added another:
1) In the ask(F("")) method, you casted the _FlashString back to a regular String to check for 0 length. The problem is that it a) doesn't compile on all platforms and b) you don't want to do a copy back to string memory just for checking. On that note, ask("") seems a hack anyways, even more so: ask(F("")). So that's why I did 2.
2) I added a method ask() that doesn't take a parameter.

Again, thanks so much for your help. Github has the update, I'll upload the package tomorrow.

Gerald

P.S.: I am doing final tests of the 1.1 firmware update. If you are interested in beta-testing, contact me through the contact form.
[Last edited Jun 20, 2016 04:03:18]
avatar
Dylan said Jun 20, 2016 04:55:24
Okay Gerald, thanks for the effort you put into incorporating these enhancements. I tried to be "minimally invasive" to your class structure which is why I went with the work-around with an empty string check. I didn't exactly like the cast but wasn't sure how you'd feel about another class member. And yeah, I suppose a major problem with trying to contribute to the code is that many of us aren't going to have all the target platforms to try!

I will contact you offline about the 1.1 beta firmware.

Dylan.
avatar
GeraldFriedland said Jun 20, 2016 15:33:38
Dylan,

No worries. I love how quickly that WordSpotter example is now.
Also, your patch with the hardware serial port will be very valuable, especially with the 1.1 firmware update that allows switching bitrates!

Gerald
Login below to reply: