I can live with those astyle settings, so I'll include them. I've been using vim: tabstop=2 shiftwidth=2 expandtabs (which is the same as --indent=spaces=2 afaict) - so that's a relief. Mixed tabs and spaces are a pitb.. While I am used to delphi and so have been using the style that puts the brace at the indent level l'm happy to go with the one you have chosen. I really dislike that style with the brace indented. While I still like much about C++, especially the template engine, I do miss delphi enums, sets and a few other features! I'll download astyle and make it nice before I push up. I've been making lots of little commits as I'm going, but will probably just squash it into one initial. I'm wondering what I should do about my instrumented function calls! For example - All my functions look like this at the moment (with XARM() and XDISARM) bool OvmsBatteryMon::checkStateChange() { XARM("OvmsBatteryMon::checkStateChange"); if (!m_dirty) { XDISARM; return false; } OvmsBatteryState newState = calc_state(); bool res = newState != m_lastState; m_lastState = newState; m_dirty = false; XDISARM; return res; } These are Macros so they would process to blank for the release. The idea is that if the object created by XARM() is not disarmed, it will cause a log to happen. (ie if there's an unexpected exception). Should I just leave them in? //.ichael On Mon, 3 Oct 2022 at 14:26, Michael Balzer <dexter@expeedo.de> wrote:
Thanks, Michael.
The main problem was the diversity of tab style.
Yes… I see you even tried to fix indentation style where it seems to have been mixed unintentionally by the module developers ^_^
Btw, I've begun including EditorConfigs and Artistic Style configs for newer modules, e.g.: - https://github.com/openvehicles/Open-Vehicle-Monitoring-System-3/blob/master... - https://github.com/openvehicles/Open-Vehicle-Monitoring-System-3/blob/master...
Regards, Michael
Am 03.10.22 um 04:07 schrieb Michael Geddes:
Pull request created.
I hope it's ok. The main problem was the diversity of tab style.
//.ichael
On Mon, 3 Oct 2022 at 09:23, Michael Geddes <frog@bunyip.wheelycreek.net> wrote:
Not 8 issues. 8 Files that I found issues in, each with a bunch of them - so around 50 all up.
I did a vimgrep for lines starting with something like 'const char* ident =' ish...
Yeah, I agree, checking for IsDefined() is a lot less cludgy, so have done that (yay for vim macros). Also the existing code didn't cope with -ve values!
I guess I better fork the project now and submit a pull request!! :) //.ichael
On Sun, 2 Oct 2022 at 22:01, Michael Balzer <dexter@expeedo.de> wrote:
Michael,
oh, the dreadful c_str() trap once again… nice find.
The deallocated heap buffer *can* have been reallocated easily, we're running on a preemptive OS and on two CPU cores.
IOW, all of these need to be fixed. Sure there are only 8 of them?
On your is_empty_metric() utility: may be I miss something, but why don't we simply use the metric's IsDefined() method?
Regards, Michael
Am 02.10.22 um 13:37 schrieb Michael Geddes:
Hi all,
I noticed some behavior on the *tpms* command I'd copied from the Kia/Kona code, which sent me down a small rabbit hole. What I was seeing was that the temperature was repeated instead of seeing the pressure and temperature.
Even in vehicle.cpp, we can see code like this:
const char* range_est = StdMetrics.ms_v_bat_range_est->AsUnitString("-", rangeUnit, 0).c_str(); if (*range_est != '-') writer->printf("Est. range: %s\n", range_est);
My C++ is a bit rusty, however I believe that the 'const char*' won't hold the temporary std::string object returned by 'AsUnitString'. To do that, I believe we need to assign to a reference (which I know will work). The problem is that the original code will mostly work (especially in cases like the above), as the deallocated heap memory won't have been reallocated.
const std::string& range_est = StdMetrics.ms_v_bat_range_est->AsUnitString("-", rangeUnit, 0); if (!is_empty_metric(range_est)) writer->printf("Est. range: %s\n", range_est.c_str());
Where I've defined in vehicle.h the following: inline bool is_empty_metric(const std::string &measure) { return (measure == "") || (measure[0] == '-'); }
Will the above work functionally? Would it be better to be:
return (measure =="") || (measure == "-")
This occurs in 8 files (outside my new Ioniq 5 files) that I have noted.
I would make a separate commit for this and do a pull request for it,
//.ichael
_______________________________________________ OvmsDev mailing listOvmsDev@lists.openvehicles.comhttp://lists.openvehicles.com/mailman/listinfo/ovmsdev
-- Michael Balzer * Helkenberger Weg 9 * D-58256 Ennepetal Fon 02333 / 833 5735 * Handy 0176 / 206 989 26
_______________________________________________ OvmsDev mailing list OvmsDev@lists.openvehicles.com http://lists.openvehicles.com/mailman/listinfo/ovmsdev
_______________________________________________ OvmsDev mailing listOvmsDev@lists.openvehicles.comhttp://lists.openvehicles.com/mailman/listinfo/ovmsdev
-- Michael Balzer * Helkenberger Weg 9 * D-58256 Ennepetal Fon 02333 / 833 5735 * Handy 0176 / 206 989 26
_______________________________________________ OvmsDev mailing list OvmsDev@lists.openvehicles.com http://lists.openvehicles.com/mailman/listinfo/ovmsdev