EmonTx4 firmware update, v1.5.4 (fix for calibration reset issue)

@TrystanLea last update was to the EEProm not the emontx4 code.

My understanding is that all happened because the EEPROM library was written for the Atmel 328P - and not changed to suit the much smaller EEPROM memory available in the Atmel AVR-DB when it was used in the emonTx V4.

Anyway, you’re off-topic. The comment I was responding to concerned configuring the temperature sensors. From the comments in several topics, the configuration commands appear to have been changed, which would explain why they don’t respond to the documented command. In the original configuration commands t0 is a prefix to the actual command and referred to all temperatures generally, prefix t1… referred to sensor 1, etc. That’s what appears to have been changed. I’ve yet to verify this, and I don’t see it happening imminently.

No, I’m don’t think I am, as the issue with the configuration not being saved was, AIUI, related to the EEProm code.

It could be there is a bug in both.

Yes I have changed the configuration options for temperature sensors in v1.5.3. The intention was to only enable temperature sensing if temperature sensors are connected at startup and so eliminate the minor interference that the temperature sensing code has on the electricity monitoring for users who do not use temperature sensors.

Unfortunately it seems that the auto detection is not completely reliable and so I need to either work out how to fix the auto detection, it might just need a delay in the code, perhaps its a power issue and I’m checking too soon? I haven’t been able to replicate the issue which makes testing a little more challenging.

The alternative solution or perhaps a worthwhile option in and of itself is to have temperature sensing set to ‘auto detect’ as default but then have the option to turn on or off temperature sensing manually.

At the moment the options saved to EEPROM are: 0: temperature sensing off & 1: auto detect temperature sensing. While you can over-ride the auto detection after startup the setting does not persist as the autodetect over-rides on the next reboot.

@NickT are you familiar with using the Arduino IDE? would you be able to set it up to upload firmware to the emonTx4? following the guide here: Firmware — OpenEnergyMonitor 0.0.1 documentation

It might be worth trying a couple of seconds of delay at Line 296 here: emontx4/EmonTxV4.ino at main · openenergymonitor/emontx4 · GitHub e.g add the line:

delay(2000);

It would be interesting to hear if that makes any difference.

1 Like

Actually I didn’t! It was already set to enabled and I didn’t power it down. I used the t1 command to enable it after power up and initialisation, when it was already sending messages. Magically the T values appeared.

I’ve never used Arduino but I’ll give it some time today and see where I get to. My shiny new HomeKit energy/temperature solution can’t fully work without it (it was built and tested using an EmonTx3 system).

After a short learning curve I did manage to compile and load the firmware with the modification you suggested. Line 296 was a blank line before the call to EmonLibCM_TemperatureEnable() and I tried it there, no change. I then moved it after the call (see image) and it works!

I’m new to the Arduino IDE (2.03 for Mac/Intel) and after following the instructions it still showed me a message that ‘newer versions of your libraries exist’, which I ignored and doesn’t seem to have come back. It also kindly pointed out that there is an `assignment within if()’ at line 1312 of emonLibCM.cpp, that although not an error might be an accident. I left this alone, @TrystanLea was that deliberate?

Do you mean this

    if (temperatureEnabled = _enable)

which is nowhere near line 1312?

If so, it’s a perfectly legal ‘C’ language construct, and it’s correct. I’ve no idea why the compiler is flagging it. Reference: Kernighan & Ritchie, The C Programming Language, pages 236 & 237.

I’m more than familiar with C thanks Robert, and the reason the compiler flags it is that it is a common programming mistake that may, or may not, be intended. Although syntactically correct, it may not be semantically correct. The original author would know that. The most recent git blame names that person as @TrystanLea so let’s hear from him. Although he could have trivially modified an earlier version of the line.

I do indeed mean line 1312 on branch avrdb (as per the instructions for compiling the firmware). See below. I ag’d the entire codebase (ag 'temperatureEnabled[\t ]*=[ \t]*_enable') and it occurs only here (and on line 44 in a comment). How nowhere near do you think it is?

BTW I’m a newcomer to this forum and am surprised at the tone of your post.

The question has been asked many times, and I’ve responded many times. Maybe you didn’t find it in a search? Is it not obvious from the context that

if (temperatureEnabled == _enable)

will not achieve the desired outcome?

The reason the line number is different is you referred to emonLibCM, where it is line 1233 in my editor, not Trystan’s -DB version - which I haven’t had time to study because all my time has been occupied making the 3V12I version to work - and this is still ongoing.

I make the following post in a constructive fashion:

Now, as and when you respond again, you can enlighten people as to why the compiler flags this as a warning. If you were responsible for that line of code you could also take the recommended course of action and add additional parentheses to show that you know what you are doing, that is using the result of an assignment statement as a truth value deliberately. If I’d seen that I’d have known, and the compiler would have kept quiet.

Making a decision about whether =, or == is correct requires knowledge of the entire context as temperatureEnabled is a global. It’s a truism to say things are

obvious from the context

but most of us don’t have time or need to have the full context in our heads and so observing language standards and coding conventions is a great time saver when different people collaborate. If you’d like any co-operation on the ‘3V12l’ version (whatever that might be) you might consider that. It would even help your future self. Here modern compilers are your friend, use them. I wish I’d had that when I first wrote C on a PDP-11 back in the day…I’ve spent countless times thinking ‘which idiot wrote it that way…’ to discover it was me!

Lastly, the magic of Git means that there are many versions of code in a single repo, it should have been obvious from the context (installing this firmware update, see what I did there?) that the libraries were on the avrdb branch.

Happy New Year
Nick

That’s great to hear! thankyou for testing that. Is there any chance that you could try reducing that delay up to the point that it stops working again? It would be good to get an idea of the amount of delay that is required.

Thank you for the input on the temperatureEnabled line. lm not personally too worried either way. I can see what you mean that it could easily confuse but also Roberts point that it does work functionally. I will discuss with Robert privately. Roberts done this work off his own back, contributed voluntarily and I know he has a lot of things going on in parallel at the moment.

Thanks again for your input and help with testing the timing modification @NickT and happy new year to you too!

Sure, I’ll get to it in a day or two. Would this delay in any way affect the calibration? Now that I’ve got it all working I’m cross-checking the outputs with my electricity meter and find that peak readings are over-reading by 30% and off-peak are under-reading by 10%. I can calibrate (need to RTFM first) but am reluctant to if it’s going to change again.

I would not expect it to change the power readings in such a significant way, the effect I have seen from enabling temperature sensing in parallel with power measurement is closer to 1% and only on CT6. That sounds like another cause to me?

Is the voltage sensor being picked up correctly? Do you see varying voltage readings?

I notice in your log from the emonTx aboove suggesting no AC Voltage sensor, is that the case when it’s installed? If it is, that is very likely to be the cause of the error that you are seeing.

Yes, perfectly legal but as @NickT points out, it’s also a common source of typo error. I’ve lost count of how many debug hours I’ve wasted over the years because I’ve accidentally typed an “=” instead of an “==” in a simple conditional test. Having the compiler ask if I really meant what I typed is a huge step forward and as Nick points out,you only have to double up on the parentheses to tell the compiler (and other readers) “written as intended”.

That construct dates all the way back to when compilers were pretty dumb at optimisation. Jamming the assignment and conditional test all into one expression made life easy for the compiler writers of the 70s. These days it doesn’t matter which form you use, so go for whatever you think is clearest. All four of these are guaranteed to produce the same behaviour…

if (temperatureEnabled = _enable)

if ((temperatureEnabled = _enable))

temperatureEnabled = _enable;
if (_enable)

temperatureEnabled = _enable;
if (temperatureEnabled)

And in the case of the AVR gcc compiler I checked and all 4 produce the exact same 3 instructions so there’s no performance difference:

a6:  80 93 06 01    sts  0x0106, r24
aa:  88 23          and  r24, r24
ac:  51 f0          breq .+20

The parameter is passed in via r24, stored in the global variable and then ANDed against itself to set the zero bit for the conditional branch.

Maybe let that be your guiding light? Choose a version that will lighten your enquiry load.

1 Like

It might be worth investigating why the delay helps, especially after EmonLibCM_TemperatureEnable() is called. That routine appears to do all the bus discovery stuff so is presumably successful at that with or without a later call to delay() - that would imply it’s not a power settling kinda’ issue.

It looks like the last thing that routine does is a global COPY_SCRATCHPAD command to store the conversion resolution, followed by its own call to delay()…

oneWire.write(COPY_SCRATCHPAD, true);
delay(20);                                      // required by DS18B20

It’s not clear where that 20 comes from, according to the datasheet that transaction typically takes 2 msecs and worst case 10, so you’d expect 20 to be sufficient:

In my ds18b20 code I poll the bus to see when the TCONV command is finished - that enabled me to write up this report on vastly varying conversion times amongst the clones. I don’t currently use the COPY_SCRATCHPAD command, but when I get time I’ll hack something up that does, and see how reliable the 2 to 10 msecs datasheet claim is across my samples.

@NickT - do you happen to know the UID of your ds18b20s?

I reduced the delay by half until I reached 5mS. The temperature sensors came up each time. I then re-loaded the original firmware (using the Admin Update function just to be sure) with the same result: the problem has gone.

How frustrating is that? What next?

They are:

  1. 28 B0 F0 75 D0 01 3C 56 So family 3C - some clone?
  2. 28 E9 A9 61 09 00 00 F0 so family 0 - the original (from the OE Shop)

I read through the post on the DS18B20, fascinating stuff. The fact that many clones put out 25 degrees in the initial phase matches the behaviour I saw on T1 (the clone) but the initial 304 that I saw on T2 (the genuine one) seems odd.

Thanks @dBC that’s a useful pointer.

That’s typical isnt it! :thinking: did you unplug the power supply, USB-C cable in between each upload, if it’s power related that would only show up after a full power cycle with a decent gap to allow any charge in capacitors and other power supply components to drain…