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.

Sign up for GitHub

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

Jump to bottom

Command should be able to end with other characters #5433

Closed
wants to merge 4 commits into from
Closed

Command should be able to end with other characters #5433

wants to merge 4 commits into from

Conversation

laurentdong
Copy link
Contributor

Description:

New implementation of ExecuteCommand()
With previous version, all the commands have to end with a space (' '). So
Var1=Mem1+1
will become an invalid command.
With this new implement, a command can be split up by not only spaces but also symbols like '=', '(', ',', etc. Actually commands can only use letters, digits and underscores, so any other characters can end the command. ("/" might be included in commands as a start of command)

There are some other improvement:

  • Leading spaces are allowed.
  • Performance should be a little bit better
  • Less memory usage

Related issue (if applicable): fixes #

Checklist:

  • [X ] The pull request is done against the dev branch
  • [X ] Only relevant files were touched (Also beware if your editor has auto-formatting feature enabled)
  • [X ] Only one feature/fix was added per PR.
  • [X ] The code change is tested and works.
  • [X ] The code change pass travis tests. Your PR cannot be merged unless tests pass

@laurentdong
Command should be able to end with other characters
4d1b3fa 
With previous version, all the commands have to end with a space (' '). So
Var1=Mem1+1
will become an invalid command.
With this new implement, a command can be splited up by not only spaces but also symbols like '=', '(', ',', etc. Actually commands can only use letters, digits and underscores, so any other characters can end the command. ("/" might be included in commands as a start of command)
There are some other improvement:
Leading spaces are allowed.
Performance should be a little bit better
Less memory usage
@arendst
Copy link
Owner

arendst commented Mar 10, 2019

I'll need to dive into this as currently it doesn't fix mqtt provided commands.

@arendst arendst added on hold by dev team Result - Feature request put on hold by member of development team Requires more research (devs) Action - Issue requires more research labels Mar 10, 2019
@arendst arendst self-assigned this Mar 10, 2019
@laurentdong
Copy link
Contributor Author

I'll need to dive into this as currently it doesn't fix mqtt provided commands.
Why? It should be able to process commands from MQTT and HTTP as well.

@stale
Copy link

stale bot commented Apr 29, 2019

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale Action - Issue left behind - Used by the BOT to call for attention label Apr 29, 2019
@ascillato
Copy link
Contributor

more time

@stale stale bot removed the stale Action - Issue left behind - Used by the BOT to call for attention label Apr 30, 2019
@stale
Copy link

stale bot commented May 25, 2019

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale Action - Issue left behind - Used by the BOT to call for attention label May 25, 2019
@ascillato2
Copy link
Collaborator

More time

@stale stale bot removed the stale Action - Issue left behind - Used by the BOT to call for attention label May 26, 2019
@stale
Copy link

stale bot commented Jun 20, 2019

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale Action - Issue left behind - Used by the BOT to call for attention label Jun 20, 2019
@stale
Copy link

stale bot commented Jun 25, 2019

This issue will be auto-closed because there hasn't been any activity for a few months. Feel free to open a new one if you still experience this problem.

@stale stale bot closed this Jun 25, 2019
@laurentdong laurentdong deleted the ExecuteCommand branch September 11, 2019 18:14
laurentdong added a commit to laurentdong/Sonoff-Tasmota that referenced this pull request Oct 1, 2019
@laurentdong
Command should be able to end with other characters
3d9ef06 
Merging of pull request arendst#5433 missed some code.
@laurentdong laurentdong mentioned this pull request Oct 2, 2019
Closed
6 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@arendst arendst

Labels
on hold by dev team Result - Feature request put on hold by member of development team Requires more research (devs) Action - Issue requires more research stale Action - Issue left behind - Used by the BOT to call for attention
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@laurentdong @arendst @ascillato @ascillato2