Tom, Thanks for your help with this. It is useful to have a different set of eyes on the problem, and see some alternative approaches.
It doesn't use any of your new metrics except battery voltage
You seem to have added: + #define MS_V_BAT_CURRENT “v.b.current" + OvmsMetricFloat* ms_v_bat_v; + OvmsMetricFloat* ms_v_bat_i; + ms_v_bat_current = new OvmsMetricFloat(MS_V_BAT_CURRENT, SM_STALE_MID); But that seems strange. Is it ms_v_bat_current or ms_v_bat_i? What is this MS_V_BAT_CURRENT? Presumably some sort of instantaneous current reading from the battery? If so, probably best to put it just after MS_V_BAT_VOLTAGE, as: #define MS_V_BAT_CURRENT “v.b.current" OvmsMetricInt* ms_v_bat_current ms_v_bat_current = new OvmsMetricInt(MS_V_BAT_CURRENT, SM_STALE_MID);
I had trouble with method overloading and hiding in the Metrics classes and ended up giving the AsStringDefault method a unique name. The api I made also seems very clunky, do I really have to make a std::string from a constant and then pass it in? Or is it more idomatic to pass in the constant and turn it into a std::string when and if it is returned?
I think the standard C++ mechanisms for providing pre-initialised default values as parameters should work, so have modified the AsString, AsInt, etc functions to use that. It doesn’t change a normal call (so current behaviour is unchanged), but does give you a way of providing a default value for undefined metrics. For example, for OvmsMetricBool, we have: std::string AsString(const char* defvalue = ""); int AsBool(const bool defvalue = false); Let’s look at the implementation of AsBool(): int OvmsMetricBool::AsBool(const bool defvalue) { if (m_defined) return m_value; else return defvalue; } So, you can call metric->AsBool(true) to make ‘true’ the default. Or metric->AsString(“undefined") to make “undefined” the default. I’ve pushed this, and I think it should satisfy your requirements. Can you modify to adopt this?
I don't like the fixed size buffer to construct the message, I copied the TPMS message code, and not knowing the "right" way to do it in this embedded C++, I just cargo culted it together. I'd be much happier if we at least used something that was aware of how big the buffer was and failed more gracefully than overflowing.
You’re right, it is ugly and dangerous. I re-worked my tpms example, to use std::string: std::string buffer; buffer.reserve(512); buffer.append("MP-0 W"); buffer.append(StandardMetrics.ms_v_tpms_fr_p->AsString()); buffer.append(",”); ... That pre-allocates 512 bytes (for performance), but can grow if that reserved size is exceeded. What do you think of that? It looks better, but I still don’t really like all those append(“,”) lines. Can you try to re-work your code, to use these three new things, and then re-submit a pull request?
I tried an SD card and it didn't work. How do I hard code or otherwise inject some startup commands without an SD card?
I’m working with the China guys on SD CARD to try to find out why it is working for them but not us. In the meantime, Steve made a ‘vfs append’ command: vfs append <quoted line> <file> You can use /store to create an event script: vfs mkdir /store/scripts vfs mkdir /store/scripts/system.start vfs append “wifi mode client <SSID>” /store/scripts/system.start/startmeup vfs append “vehicle module NL” /store/scripts/system.start/startmeup vfs mkdir /store/scripts/network.up vfs append “server v2 start” /store/scripts/network.up/startmeup Untested, so may not be perfect, but I think you get the idea…
Since I rebased onto the current master, things don't compile:
/vagrant/Open-Vehicle-Monitoring-System-3/vehicle/OVMS.V3/components/spiffs/./spiffs_vfs.c: In function 'vfs_spiffs_fstat’:
I think this is related to the sdkconfig. Anyway, I removed the components/spiffs (as we don’t use it), and it now compiles. I also created a sdkconfig.default that developers can copy to sdkconfig, to provide a good set of defaults. You should also be able to diff it, to see anything changed. Regards, Mark.
On 9 Oct 2017, at 6:10 PM, Tom Parker <tom@carrott.org> wrote:
On 09/10/17 03:29, Mark Webb-Johnson wrote:
Do the best you can, and if you can’t work out how to get something in particular, let me know or leave it as a “TODO” marked in the code.
Thanks for adding the metrics. I sent a pull request with an initial implementation of the Stat message. It doesn't use any of your new metrics except battery voltage (I'd already added that one) and I wedged sending it into a nearby loop which is almost certainly the wrong place, but it works and I was able to send telemetry from a real car today using a wifi hotspot for network connectivity.
I had trouble with method overloading and hiding in the Metrics classes and ended up giving the AsStringDefault method a unique name. The api I made also seems very clunky, do I really have to make a std::string from a constant and then pass it in? Or is it more idomatic to pass in the constant and turn it into a std::string when and if it is returned?
I don't like the fixed size buffer to construct the message, I copied the TPMS message code, and not knowing the "right" way to do it in this embedded C++, I just cargo culted it together. I'd be much happier if we at least used something that was aware of how big the buffer was and failed more gracefully than overflowing.
I tried an SD card and it didn't work. How do I hard code or otherwise inject some startup commands without an SD card? I tried tracing through the command code but quickly decided it was easier to just start the vehicle module, wifi and v2 server with my laptop in monitor mode.
Since I rebased onto the current master, things don't compile:
/vagrant/Open-Vehicle-Monitoring-System-3/vehicle/OVMS.V3/components/spiffs/./spiffs_vfs.c: In function 'vfs_spiffs_fstat': /vagrant/Open-Vehicle-Monitoring-System-3/vehicle/OVMS.V3/components/spiffs/./spiffs_vfs.c:344:22: error: 'CONFIG_SPIFFS_LOG_PAGE_SIZE' undeclared (first use in this function) st->st_blksize = CONFIG_SPIFFS_LOG_PAGE_SIZE;
_______________________________________________ OvmsDev mailing list OvmsDev@lists.teslaclub.hk http://lists.teslaclub.hk/mailman/listinfo/ovmsdev