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

startStreaming() using Stream class #9

Open
embedded-creations opened this issue Jul 13, 2021 · 14 comments
Open

startStreaming() using Stream class #9

embedded-creations opened this issue Jul 13, 2021 · 14 comments

Comments

@embedded-creations
Copy link

embedded-creations commented Jul 13, 2021

I'm trying to add YMODEM receive to my project using this library (my fork which allows for setting the Stream port instead of using hardcoded Serial). I can't use the existing streaming options in Commander directly, as the library depends on receiving data via a Stream. I don't really want to modify the library just to work with Commander. I imagine there are more libraries using Stream that could be included in future prefabs, so I'd rather find a good solution to this problem than hack together support for one library to solve my immediate problem.

I see two options here:

  1. Make a mode where Commander gives up control of inPort, and lets other code use it until stopStreaming() is called
  • This wouldn't work with !dataStreamMode as the other code could read from the Stream before Commander gets a chance to look for 0x04
  1. Use PairedPipedStream from this library (my fork where I modified it to optionally output to a Stream on one end automatically). Commander can inspect each byte, and load it into one end of a PairedPipedStream. The other code can read from the other end as if it were reading directly from inPort (though Commander decides when to stop streaming data), and whatever it writes gets automatically written to outPort.
  • This uses more memory as the PairedPipedStream has a fixed buffer, but with a little extra code we could dynamically resize the buffer.
  1. I guess there's a third option where there's no modification to Commander, and I just don't call cmd.update() to make option 1 work inside of my sketch, or I take care of option 2 inside a special handler in my sketch/prefab.

I only thought of the much simpler option 1 after doing all the work upgrading PairedPipedStream and prototyping option 2 and getting YMODEM receive to work. At this point I think the remaining work for option 1 and 2 are about the same.

LMK if you have some thoughts on these options, or want more details on anything

@embedded-creations
Copy link
Author

Thinking about this more: I think the code/ideas from option 2 should be integrated into Commander, so when streaming mode is enabled, the code using streaming can use Cmdr.read() and Cmdr.write() etc.

@embedded-creations
Copy link
Author

It was surprisingly easy to add this feature. I barely used any of the code I wrote over the last day and a half, but used plenty of the knowledge I had gained by doing that otherwise useless exercise.

embedded-creations@7f41e29

embedded-creations@092d216

I can now use Cmdr.read() and Cmdr.write() with the XYmodem library and transfer files to my SD card.

I deleted a lot of print*() and write() methods which were causing either compilation or runtime problems and seemed to be unnecessary after inheriting from the Print class, but only tested a few menus in one sketch, so I may have overlooked some important feature.

@CreativeRobotics
Copy link
Owner

THanks, that looks great. I will try and find some time over the next week to have a good look through and make sure I've understood it all properly, then look at how to integrate it all.

Incidentally, I have just figured out a way of adding an extra help system, I'm just testing it out. I added a char array pointer so you can add an array of strings and changed how internal help commands are handled. If the internal handler sees that the help command has a payload it will check if the payload is a command and if so it will try and print out the contents of the extra help array, at the index that matches the command. The extra help array needs to have the same number of elements as the command list, and I haven't made it work with multiple command sets yet, but if gives users a way of adding more detailed help text - if you have a command called 'configure', you can send 'help configure' to get the detailed help file for that command.

@embedded-creations
Copy link
Author

The extra help system seems like a great feature. In the meantime while you look at my prototype code I'll clean up my prefab and share it so you can see how a prefab (with streaming) would look and work with the new structure

@embedded-creations
Copy link
Author

I pushed an unfinished (but working) PrefabFileExplorerFS example to my branch, showing how a prefab might look with the new structure

embedded-creations@80c50eb

Next up is to see if I can integrate support for navigating two filesystems just by adding two instances of the same class to the sketch

@embedded-creations
Copy link
Author

What development board did you use for creating your PrefabFileExplorer example?

@CreativeRobotics
Copy link
Owner

I think it was probably an Adafruit Feather M0 Adalogger.

@CreativeRobotics
Copy link
Owner

I've been playing with your new version that inherits from Stream. It works perfectly so I will probably go ahead and integrate that into a new release along with the extended help system before anything else.
I'm testing out the new help system and it works really well. I've also worked up a couple of advanced examples that manipulate the text and function pointers in the command array, potentially a very powerful feature. One example has a 'hello' command and a 'swop' command. The swop command actually changes the hello command to use a different handler, and changes the command string, help and extended help text pointers (it swops to a French equavalent of the hello command and help text).
I've got another example that lets you use String objects for the help, command and extended help text by assigning the pointer returned by String.c_str() to them. The example lets you re-write the command text, help text and extended help text for the first command in the list.
I hadn't considered any of this before, but by declaring the command array non const all these things can be changed, and it opens up the possibility of creating an entire command array at run time. I am imagining a scenario where a basic command set is used to open a file from flash or SD card that then uses those commands to generate a new bespoke command set, possibly in a different language as well ... I might explore this over the weekend.

@embedded-creations
Copy link
Author

Glad it's working for you. Making the array non-const does open up more possibilities but we'd want to make sure it doesn't cause extra RAM usage by putting const strings into RAM or using extra RAM/Flash for strings in a class that is instantiated multiple times.

I should mention CommandCollection isn't necessarily the best name, and it doesn't match the capitalization style you use, so I hope you don't just accept that as is.

BTW I see from your LinkedIn profile we're not too far away, I'm in Cambridge. (I have no idea what my profile says, I don't keep it up to date)

@CreativeRobotics
Copy link
Owner

Yes - Its an option for people to use if they want and probably best for top end microcontrollers with lots of RAM, and I'm putting these examples in a folder called 'advanced'.
I was going to suggest CommandSet instead, mostly for brevity. I capitalise the first letter of classes, but not functions, structs or variables, so what you have done does seem to match...
Your linkedin profile says New York, so Cambridge is quite a bit closer!

@CreativeRobotics
Copy link
Owner

I've just pushed some of these updates to the repo. I'm not issuing it as a release yet, but let me know what you think. I have included a working example of a dynamically created command list.

@embedded-creations
Copy link
Author

I skimmed your commit but didn't have time to really look at any examples over the last few days. I'm focusing on adding LittleFS compatibility to my premod with the little free time I have right now. LMK if there's any specific example you want me to check out.

@embedded-creations
Copy link
Author

embedded-creations commented Jul 20, 2021

I'm finally taking a look at your latest commits and examples in a bit more detail. Some questions and comments as they arise:

  • "swop" is not a term I'm used to seeing, and looks like a typo though I see it's actually correct. I suggest changing it to "swap" to avoid confusion, especially as the example name is functionswap not functionswop. https://english.stackexchange.com/questions/21228/is-swop-an-acceptable-variant-of-swap
  • Being able to change functions at runtime does seem very useful, and I immediately found a use for it trying to simplify my prefab
  • const char* issues:
    • String.c_str() returns a const char*, not a char *
    • You shouldn't cast a const char* to a char *
    • Commander receives and stores char* in several places where I believe const char* is more appropriate. e.g. extraHelp could be const, as it's just printed, not modified.
    • Changing unnecessary char* to const char* would help get rid of some warnings I've been seeing in my sketch but ignoring: warning: ISO C++ forbids converting a string constant to 'char*'. I use "Compiler Warnings": All in my Arduino settings to avoid missing important warnings.
    • I modified the functionswap example to use only const char*, and now it's cleaner as there's no more casts. I changed a few places in Commander.h, and now the example compiles without warnings and still works. Changes below:
 typedef struct commandList_t{
-	char* commandString;
+	const char* commandString;
   cmdHandler handler;
-	char* manualString;
+	const char* manualString;
 } commandList_t;

-	Commander&	 setExtraHelp(char* ptr[])				{extraHelp = ptr; return *this;}
+	Commander&	 setExtraHelp(const char* ptr[])				{extraHelp = ptr; return *this;}

-	char** extraHelp;
+	const char** extraHelp;

@CreativeRobotics
Copy link
Owner

CreativeRobotics commented Jul 21, 2021

Yes - I need to be better at removing my own ideosyncracies from finished code.
I reinstalled windows a few weeks ago and haven't quite got around to setting all my software up as it was before, including enabling warnings in Arduino. I've made all the changes you pointed out, will push out another small release later. Thanks again.

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