Fix/refactor date time #1549
Fix/refactor date time #1549
Conversation
Moved from Services to SmingCore Rationalised conversion method names to... from... Marked previous named methods as deprecated Updated dependant libraries
Fixes fromHttpDate to work with long names in day / month arrays. Use format method for all string convenience methods. Rationalise comments. Remove "??" from unimplemented format options. Add sample project.
Add day of year. Add more examples to sample.
|
@riban-bw Don't forget to run |
Adds localisation framework (default en-gb). Fixes 2 digit date check (was <69). Improves documentation for format method. Corrects typo.
I have downgraded Doxygen and Clang in Cygwin to one that works so I can now run clang-format. Hopefully this should improve my ability to submit changes within Sming coding style guidelines. I may even be able to beat my IDE into submission to produce the required style to begin with! |
Fixes coding style.
|
I sometimes hate git - grrrrrr! The hard part should be writing the code not committing it to version control. Hopefully it looks in better shape now. |
That file has to be taken out of the PR :).
I guess I was not clear enough. But please DON'T use percentage Something (%S) as format. Use "S". Please, see the format parameters for PHP for example and follow them http://php.net/manual/en/function.date.php#format .
Locale for an embedded framework should be disabled by default and enabled only if needed. Locales tend to have a lot of long strings, formatting settings and etc settings that are taking space. |
Sming is a C++ framework. I think the strftime format may be familiar to C++ programmers. It is also more flexible than the PHP implementation - normal text may be included within the format string, e.g. "Time: %X". (I prefer Sming to Arduino because Sming implements printf which is an invaluable feature.) I understand that someone who is familiar with PHP may prefer the PHP format, someone who is familiar with Python may want the Python format, etc. but I recommend implementing the % format for all those C++ programmers who are already familiar with it. We could implement a similar function with different formatting if you want (under a separate PR). This could be designed to avoid overheads due to the way that only used functions are added to the firmware. Regarding locale - this is implemented mostly with those horrid pre-compiler directives which ensure only the required bits are put into the code, e.g. I agree that we need to constrain firmware size with particular attention to each type of memory and do try to code with an embedded ethos, though may not create optimal code on a first attempt. (That is what peer review is for - right?) On the point of bloat, I was considering adding a feature request to measure libsming size after each commit and compare with previous / develop head, so that we can monitor the change in size that each commit has which may help to keep a handle on bloat. |
Do you know a free web service where we can send the data and it will generated the needed graphs for us? It is good to see how data size, free heap size and code size vary. |
I agree with you. Let it be One more point - make sure that the code passes CI tests. At the moment it seem to fail under Travis due to coding style issues and under Windows due to 'setlocale' is not a member of 'std'
char* __old = std::setlocale(LC_NUMERIC, 0);
^ |
|
I can see that the coding style issue relates to indentation I added to Locale.h to make things easier to understand and edit. I will fix that (and also fix some errors with German I was able to identify when chatting to a German colleague). The Windows issue may be more tricky. I looks like something peculiar to Chocolatey which I cannot install on my development machine (the only Windows machine I have access to). This error does not occur with cygwin or Ubuntu. I suspect that the use of Unicode characters in some of the languages may be triggering xtensa libraries to kick in locale elements which fail for Windows. This may be a dormant bug in their software. I will look at the errors from appveyor and try to diagnose but this may prove challenging. If anyone with a Chocolatey install is available to help diagnose and test this it would be useful. |
I guess the issue is more trivial. On Windows the files names are case insensitive. Which means that your Locale.h will be loaded when locale.h. Try renaming it to something like CoreLocale.h and see if that works. |
|
Thanks slaff, I will try that. (I configured my Windows machine to be case insensitive to allow me to build xtensa. Maybe I should change it back to pick up stuff like this.) I was about to commit these changes but realised I had started to implement the last couple of formats (variations on week number) but not yet tested so need to complete that before I commit. Hopefully sort in next few days. (I didn't have to change the German - I had got it right first time! Native German speakers feel free to contradict me.) |
The months and the days of the week are correct. If there is something more I can check it too. |
|
The API documentation has a table showing which parameters are influenced by locale. I guess I could change the locale of a test machine and compare Sming output with a native test app. (I think this can be done with an environmental variable in Linux so I could probably create a test for this for the supported languages.) I have been doing similar with cygwin but its implementation of strftime seems simplistic and does not include all the options so firing up a Linux machine may be a better test environment. The only other locale element I set for Germany is the short date where Germany uses the dot separator. |
Corrects coding style. Renames locale header to avoid conflict with locale.h on case insensitive file systems.
|
Don't merge yet - some minor changes required for documentation and locales. |
Update API documentation. Corrects some locale errors. Update sample to match library.
That unneeded modification is still in the PR. Remove it using: After that the older version of the file will be ready to commit to this PR. |
|
Thanks for the tip @slaff. I used to list all the changed permission files and found a few more so corrected those. Knowing the ID of the version to compare to was handy. I have just seen in github where to find that so maybe I can be more efficient with git in future! Life was so much easier with subversion :-). |
|
@riban-bw Thanks for the PR. It took a bit longer but hopefully you are now better prepared |
SmingHub#1549 Initial commit as ascii (probably Windows) e9 SmingHub#1715 replaced with UTF-8 'replacement character' ef bf bd. UTF-8 code is ce a9 Best to stick with original encoding, UTF8 is likely to cause problems with existing code
The french and german strings defined in `SmingLocale.h` appear to have been corrupted. Taking the é in février as an example: PR #1549 Initial commit as ascii (probably Windows) `e9` PR #1715 changed to UTF-8 'replacement character' ef bf bd. The UTF-8 code would be ce a9 but as that's two bytes it could break existing code. Best to stick with original encoding.
In response to issue #1545 this PR refactors DateTime to rationalise method names, deprecating old (inconsistent) names.
Moved from Services to SmingCore.
Also adds format method which returns a string based on formatting, providing strftime formatting of string. (Some elements not implemented but mostly there.)
Provides a sample project demonstrating DateTime string output features.
Fixes bug with toHttpDate not setting day of week.
Changed convenience string methods to use new format method.
Not (yet) implemented any locale - all currently in English with existing variation in date formats, e.g. US date format with dot separator used by some European countries.