Opaygo step by step librairies and functions integration #100
Conversation
|
Hi @dmohns, i would like to know how to organize the functions and libraries of Opaygo that we are going to use? Two possibilities are available to us: Personally I suggest that we opt for the first possibility to have a more structured and more organized code. But for now I'm waiting for your answer to organize the rest of the code. THANKS |
Hey @Peguy-WANDA sorry for the late reply 🙈 Unfortunately, I'm not sure I understand the question. Let me clarify: The Moving forward, the code will entirely disappear from the OpenSmartMeter repository and will be pulled in via dependencies of In case you are talking about "wrapper" functions that are needed in OpenSmartMeter firmware to utilize PAYGO functions: These function can basically go wherever you see them fit best. I, personally, would probably put it to a similar place where the current token functions sit, i.e. |
|
Another comment. It looks like your branched out from a very old version of the 
(Blue in the screenshot) Could you rebase or update your branch to the latest version of |
…n' of https://github.com/EnAccess/OpenSmartMeter into Opaygo-step-by-step-librairies-and-functions-integration
| @@ -29,7 +29,6 @@ uint64_t ConvertHashToTokenExtended(uint64_t this_hash) { | |||
| } | |||
|
|
|||
| uint64_t GenerateOPAYGOTokenExtended(uint64_t LastToken, char SECRET_KEY[16]) { | |||
|
|
|||
There was a problem hiding this comment.
Can you revert that change, so the files is untouched?
| @@ -9,6 +9,7 @@ | |||
| #include "opaygo_decoder_extended.h" | |||
|
|
|||
|
|
|||
|
|
|||
There was a problem hiding this comment.
Can you revert that change, so the files is untouched?
| @@ -45,3 +45,4 @@ lib_deps = | |||
| thingsboard/ThingsBoard@0.6.0 | |||
| vshymanskyy/TinyGSM@0.11.5 | |||
| https://github.com/cyberp/AT24Cx.git | |||
| Wire | |||
| @@ -10,3 +10,4 @@ DerivePointerAlignment: false | |||
| # Always break after if to get accurate coverage | |||
| AllowShortIfStatementsOnASingleLine: false | |||
| AllowShortLoopsOnASingleLine: false | |||
|
|
|||
There was a problem hiding this comment.
Can you revert that change so the file is untouched?
| #pragma once | ||
|
|
||
| //#pragma once | ||
| #ifndef credit_hpp | ||
| #define credit_hpp |
There was a problem hiding this comment.
The two syntaxe are doing the same things but there is a difference between them.
The #ifndef/#define/#endif trick works on any C compiler, and on some of them it speeds up the compilation process. The #pragma trick is non-standards, and only works on few C compilers, and may result in different semantics in those that do not support it.
When I use #pragma trick, I have many warning, but with #ifndef/#define/#endif trick the warnings disappear
There was a problem hiding this comment.
but for this PR I'm just going to put back #pragma once as you wanted
There was a problem hiding this comment.
Hey, few comments on the PR, would be great if you could address them
- Could you revert all whitespace changes to files that are unrelated to this PR? If you think any of those changes would be better for code readability please feel free to send a separate PR.
- I see you are changing all include guards from
#pragma onceto#ifndefany specific reason why you need this? As above, please leave these untouched in this PR, but open another one, if you think this will make the code better.
Other than that, looks good to me!! Thanks for all the work
| delay(20); | ||
| lcd.clear(); | ||
| buss(); | ||
| lcd.clear(); | ||
| lcd.setCursor(0, 0); |
There was a problem hiding this comment.
I don't know what the buss() function does. Could you explain why it is needed for lcd.clear() to be called after buss() instead of before?
There was a problem hiding this comment.
Ok I will add comments in the code to explain it
There was a problem hiding this comment.
The position of lcd.clear() in relation to buss() has no impact on the code, before or after it doesn't change anything and besides I hadn't even noticed that I had indeed changed it
There was a problem hiding this comment.
The buss() function is there just to emit a sound to signal to the user that he can already enter the token in sts mode
|
Daniel I think as of now everything is good for this pull request, thank you for your comments, feel free to leave more if needed. |
| #define ACTIVE_UNTIL_NVRAM_ADDRESS \ | ||
| 0 // 56 bytes address from 0 to 55 in the NVRAM | ||
| #define TIME_INITIALIZATION_NVRAM_ADDRESS 4 |
There was a problem hiding this comment.
Where are these used? If they aren't, can we remove them?
| // Device parameters location in Flash/EEPROM | ||
| unsigned int TokenCount_eeprom_location = 6; | ||
| unsigned int UsedTokens_eeprom_location = 8; | ||
| unsigned int PAYGEnabled_eeprom_location = 10; | ||
| unsigned int ActiveUntil_eeprom_location = 11; | ||
| unsigned int TokenEntryLockedUntil_eeprom_location = 15; | ||
| unsigned int LAST_TIME_STAMP_ADDRESS = 27; | ||
| unsigned int NB_DISCONNECTIONS_ADDRESS = 32; |
There was a problem hiding this comment.
What do you think about making some like a "global" EEPROM map? We don't have to do this now in this PR, but as an idea?
For example, in global_defined we could include all the EEPROM locations as macros?
| uint32_t GetTimeInSeconds() { | ||
| /* | ||
| *Returns the unixtime in seconds of Now | ||
| */ | ||
| DateTime now = rtc.now(); | ||
| uint32_t nowInSeconds = | ||
| (now.unixtime() - | ||
| timeInitializationRtc); // we substract the init time so that it's | ||
| // easier to read when debugging, and works the | ||
| // same as Arduino Time mgt | ||
| return (nowInSeconds); | ||
| } |
There was a problem hiding this comment.
This isn't PAYGO related, but time related, should it go in time_management.hpp instead?
| void AddTime(int ActivationValue) { | ||
| uint32_t Now = GetTimeInSeconds(); | ||
| int NumberOfSecondsToActivate = (ActivationValue * 3600 * 24) / TIME_DIVIDER; | ||
|
|
||
| if (ActiveUntil < Now) { | ||
| ActiveUntil = Now; | ||
| } | ||
|
|
||
| ActiveUntil += | ||
| NumberOfSecondsToActivate; // We add the number of days (converted in | ||
| // seconds for to compare to our RTC time) | ||
| } | ||
|
|
||
| void SetTime(int ActivationValue) { | ||
| uint32_t Now = GetTimeInSeconds(); | ||
| int NumberOfSecondsToActivate = (ActivationValue * 3600 * 24) / TIME_DIVIDER; | ||
|
|
||
| ActiveUntil = | ||
| Now + | ||
| NumberOfSecondsToActivate; // We set the number of days (converted in | ||
| // seconds for to compare to our RTC time) | ||
| } |
There was a problem hiding this comment.
This isn't PAYGO related, but time related, should it go in time_management.hpp instead?
| void initializeTime() { | ||
| if (!rtc.begin()) { | ||
| Serial.println("Couldn't find RTC"); | ||
| while (1) | ||
| ; | ||
| } | ||
|
|
||
| if (!rtc.isrunning()) { | ||
| Serial.println("RTC is NOT running!"); | ||
| } | ||
|
|
||
| // We check that it is not a disconnection power - Arduino | ||
| uint32_t nvramCheck = readUint32FromNvram(TIME_INITIALIZATION_NVRAM_ADDRESS); | ||
| if (nvramCheck == 0) { // it is the first setup of the Arduino | ||
| rtc.adjust( | ||
| DateTime(F(__DATE__), F(__TIME__))); // sets the RTC to the date & time | ||
| // this sketch was compiled | ||
| DateTime now = rtc.now(); | ||
| timeInitializationRtc = now.unixtime(); | ||
| storeUint32InNvram(timeInitializationRtc, | ||
| TIME_INITIALIZATION_NVRAM_ADDRESS); | ||
| } else { // a disconnection happened | ||
| timeInitializationRtc = nvramCheck; | ||
| incrementNbDisconnectionsEeprom(); | ||
| storeTimeStampEEPROM( | ||
| GetTimeInSeconds()); // to avoid counting twice the disconnection if | ||
| // the previous TimeStamp was too long ago | ||
| } | ||
| } |
There was a problem hiding this comment.
This isn't PAYGO related, but time related, should it go in time_management.hpp instead?
| #define NON_ACCEPTED_KEY -3 | ||
|
|
||
| #define TIME_DIVIDER 1 | ||
| // #define RESTRICTED_DIGIT_SET_MODE |
dmohns
deleted the
Opaygo-step-by-step-librairies-and-functions-integration
branch
* git changes * Changes in loop * change on token_management * main.cpp * include opaygo_decoder * token_management * add function GetDataFromToken from opaygo_decoder * this commit is only to update the branch * update branch * add opaygo_functions * add LoadActivationVariables() function * complete main.cpp with functions of opaygo * add UpdateDeviceStatusFromTokenValue() * add functions in opaygo_functions * update code * add functions in opaygo_functions * solving problem of time * editorConfig problem solving * editorConfig problem solving * editorConfig problem solving * clang-format problem solving * EditConfig problem solving * EditConfig problem solving * commit before changing branch * solving conflict for PR * format document * format document --------- Co-authored-by: Peguy WANDA <peguy@debian-BULLSEYE-live-builder-AMD64> Co-authored-by: Daniel Mohns <daniel.mohns@posteo.de>
* git changes * Changes in loop * change on token_management * main.cpp * include opaygo_decoder * token_management * add function GetDataFromToken from opaygo_decoder * this commit is only to update the branch * update branch * add opaygo_functions * add LoadActivationVariables() function * complete main.cpp with functions of opaygo * add UpdateDeviceStatusFromTokenValue() * add functions in opaygo_functions * update code * add functions in opaygo_functions * solving problem of time * editorConfig problem solving * editorConfig problem solving * editorConfig problem solving * clang-format problem solving * EditConfig problem solving * EditConfig problem solving * commit before changing branch * solving conflict for PR * format document * format document --------- Co-authored-by: Peguy WANDA <peguy@debian-BULLSEYE-live-builder-AMD64> Co-authored-by: Daniel Mohns <daniel.mohns@posteo.de>
Add in main.cpp the function GetDataFromToken(...) from opaygo_decoder.h