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

Develop #102

Merged
merged 11 commits into from
Sep 24, 2023
Merged

Develop #102

merged 11 commits into from
Sep 24, 2023

Conversation

mhamidjamil
Copy link
Owner

@mhamidjamil mhamidjamil commented Sep 22, 2023

#96 #97 #100 Module can now use SPIFFS (its own memory) for Create, Read and Update variable data. so, #67 is not necessary now and #68 will be linked with this feature in coming days.

* Update v5_stable.ino

this commit include two major changes:
1: New part added to update/add variable value in SPIFFS file
2: Variables and functions name are changed:
    a) global variable name are in snake case
    b) functions name are in camel case
    and from now local variables name will be in camel case
creating new branch as I'm expecting errors as i changed lot of variables and functions names.

* Module can now read/write variables in SPIFFS
@mhamidjamil mhamidjamil added enhancement New feature or request new feature Add new feature labels Sep 22, 2023
@mhamidjamil mhamidjamil added this to the New Feature in v5 milestone Sep 22, 2023
@mhamidjamil mhamidjamil self-assigned this Sep 22, 2023
@wasey-rao
Copy link
Collaborator

I was looking into you previous PR #101 seems perfect but my suggestion is to use same type of SPI memory like you recently used littelFS in ESP32-S3 project and now you are using SPIFFS in this project which is good w.r.t learning point but we have to work on #27 like if you write a function for this project it is not an easy task to transfer that function to other projects which are using other type of SPI memory type, so my point is use same type of SPI memory so your functions related to SPI memory can be useable in any project.

} else if (command.indexOf("readSPIFFS") != -1) {
println("Data in SPIFFS : " + readSPIFFS());
} else if (command.indexOf("value of:") != -1) {
String varName =
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • You have read and update variables but you are not using those yet, use at least 2 SPIFFS variables.
  • this part is defined as a function I'll suggest to use it to reduce complexity

@@ -1731,3 +1689,125 @@ String get_HTTP_string(String message) {
}
return tempStr;
}

bool newPackageSubscribed(String str) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • You have not defined these functions before void setup() where other functions are defined
  • This function should also update a variable in SPIFFS

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • This function should also send message to user that his package is also understanded by module.

Copy link
Collaborator

@wasey-rao wasey-rao left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In new version 5.5.0 you should be following these points:

  1. Module should also create/update variable in which it store the expiry date of package
  2. Module should be using SPI variables in real time and updating there values according to scenario
  3. Module should use expiry date of package:
  • If package will expire after one day remind user (by whatsapp) to purchase any message package
  • If package is already expired then don't send or forward any message, just use whatspp.

All these mentioned features must be finalized in v5.5 so we finalize this version as soon as possible.

@wasey-rao
Copy link
Collaborator

in this 1c66f30 I got two issues:

  1. It automatically set time, not sure as it send message
  2. ThingSpeak issue ThingSpeak Upload issue #103 (Fix it on higher priority)

@wasey-rao
Copy link
Collaborator

as whatsapp messages are working and i created new issue regarding to thingspeak and messages package so merging this PR

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request new feature Add new feature
Projects
Development

Successfully merging this pull request may close these issues.

2 participants