Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
add Low-Power demo #6989
add Low-Power demo #6989
Changes from 1 commit
0c870ae
fd1c366
b912664
287bc2d
7ea68d8
f77a2c5
387e30d
0db9254
3f3ab30
df75200
ab30950
11f1931
7d6a03b
2003ef4
acf3cec
2f2c508
47b73a2
36a8dbb
7178362
5d16fca
7a74da2
aa4feb2
8f4448d
b8e42e4
068f4bd
3118de5
ac40b6e
662b1f2
5695626
7627080
7b0e515
a41c381
b93c2e3
f4d00c6
841fa1b
efe55a0
ebc2b0a
c9c073a
222b9e8
9ef40f8
3ffb8ec
b902ed2
6ff566d
c8a5955
d9ea100
431f5dc
9597a12
7d3ee8d
701634e
b618e6c
3292f9b
1e80279
93a527b
6018d0b
55d95e2
b990e59
c107c43
fecacfd
601a5ae
a3ddde1
535d483
3775b1b
56cb239
c26ce4a
a76ec12
a79f48c
ee4c701
d378102
df1a494
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's encapsulate the functionality related to the rtc and the data you're saving.
First, let's list what I consider as "related functionality":
updateRTC()
)//Read struct from RTC memory
resetCount = someNumber
)Encapsulation in C++ means putting the functionality in an object, but in such a way that using that object hides the ugly details, and simplifies the code around the line where it is being used from.
A C++ object is essentially a group of functions that share a scope of specific variables. The functions are usually referred to as either methods or member functions, and the variables in the internal scope are called member variables or just members (this is a simplification).
There are 2 types of C++ objects: class and struct. They are functionally the same, the only difference between them is the default visibility of the object's internal variables:
In this example, you're already using a struct. Because this is an example sketch, use of a struct is ok.
In C++, a struct is very much like a normal-C struct, but you can add methods to it, and that is how the functionality listed above will be encapsulated.
Given the above, the following is one possible way of encapsulating:
Now, usage can be as follows:
In the above you will notice use of
this
. Within any class method,this
is a special member variable: a pointer to the class instance you are in. So, in this case it's the memory address of the start of the global rtcData struct instance.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can sort of follow that part, but I'd rather minimize refactoring the code into C++ as I can't read it (and can't maintain it). About twice as many people know/use C as know C++, so re-writing it into C++ makes it incomprehensible to those of us that don't do C++. An 'example' or 'demo' should be understood by the largest number of users, not just the C++ crowd.
The other refactor (several comments below) makes perfectly good sense though. I'm all for making it more readable. Working on it...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If I'm reading your code correctly, I think the definition in the struct needs to change since resetCount is now an int, so
should be something like
with matching revisions to readFrom and writeTo
edit: dunno if that's going to work, as the crc32 looks like it's operating byte-wide
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This resetCause isn't needed, the line should be removed. You are declaring it inside setup(), and using it only there, so that local declaration is enough.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You're right, I'd forgotten I moved that whole thing inside of setup(). Removed.
For whatever reason, I got exceptions if I tried to do multiple reads of ESP.getResetReason() like this:
I didn't try to hunt it down, as it was a very long stack dump. I've never seen a stack dump that long. That's why I read it once into a string.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see we already have this crc32 function in crc32.cpp, the declaration is in cores/esp8266/coredecls.h. Please reuse that one to not reinvent the wheel.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It may take me a week to figure that one out, as it's not exactly clear how to use it for a newbie, nor is it listed at readthedocs. The only example I could find that used it is equally unclear (WiFiShutdown.ino). At the moment I'm trying to figure out what this is doing, quite literally symbol-by-symbol.
nv->crc = crc32(&nv->data, sizeof(nv->data));
From what I've figured out so far, that's called "function call by reference" but giving it a name doesn't magically make me understand it. I suspect that's why the person that wrote the RTCUserMemory example coded their own version of a crc32 (that's what I copied).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done, replaced with the one linked from coredecls.h. That was less painful than I thought.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This would get replaced by the RTCData::writeToRTC() method, declared and defined within the class at the start of the sketch.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if-else bodies on their own lines, please, like so:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Already fixed, just not committed yet. I saw the error at Travis.