What is wrong with my driver?

[Jan-Jaap van der Geer]

I've modified an existing driver that originally only captured LOOP data but I have added a 'genArchiveRecords()' method to it that fetches data from a REST interface. This seems to work fine and it fetches the data.

However, since this modification it no longer fetches the LOOP data. The driver sees the data and according to some logging it passes it to weewx just fine, but for some reason weewx seems to be silently ignoring the data, it is never added to the database with no indication why in the logfile.

I've been investigating this for some days and I don't understand why it is happening. I have found out that it is related to the genArchiveRecords() method: If I have this method like this:

    def genArchiveRecords(self, since_ts):

        yield from ()

then weewx is ignoring all values returned by genLoopPackets().

However, if I comment out the genArchiveRecords() alltogether, then suddenly the records returned from weewx by genLoopPackets() are being inserted in the database as expected.

This does not make any sense to me. Anybody understands what is going on?

The code for this driver can be found here: https://github.com/jjvdgeer/weatherflow-udp

Thanks,

Jan-Jaap

[gjr80]

It might be worth revising the operation of genLoopPackets() and genArchiveRecords(), in particular when running under hardware and software record generation.

The task of genLoopPackets() is to emit loop packets and these packets are processed and accumulated by WeeWX. The accumulated loop packets can be used to (1) synthesise an archive record at the end of the archive interval (which is ultimately saved to database in the archive table) and (2) if loop_hilo is True update the daily summary tables. On the other hand the task of genArchiveRecords() is to emit, when asked,  archive records which are processed by WeeWX and ultimately saved to database in the archive table and used to update the daily summary tables.

If you are using software record generation then WeeWX never asks the driver to emit archive records, instead WeeWX takes the accumulated loop packet data received during an archive interval and synthesises an archive record. This archive record is saved to database in the archive table.If loop_hilo is True the daily summary tables are updated using the accumulated loop data. In other words the driver just continually emits loop packets and WeeWX does the rest.

When using hardware record generation the driver continues to emit loop packets, and WeeWX continues to accumulate them; however, at the end of the archive interval instead of synthesising an archive record WeeWX asks the driver for an archive record. Since the driver has a genArchiveRecords() method an archive record is emitted by the driver and WeeWX takes that archive record and ultimately saves it to database in the archive table. WeeWX will use the archive data and, if loop_hilo is True, the accumulated loop packet data, to update the daily summary tables (note that no archive record is ever synthesised by WeeWX, the only archive record that exists comes from the driver). There is one more aspect to WeeWX operation when hardware record generation is selected. WeeWX asks the driver for an archive record by calling the genArchiveRecords() method,  if that call fails then WeeWX falls back to software record generation (ie WeeWX synthesises an archive record from the accumulated loop packets). So even though you have set hardware record generation WeeWX will still function properly with a driver that does not support hardware record generation (ie it has no genArchiveRecords() method).

Back to your situation. When you have genArchiveRecords() enabled you say that "The driver sees the data and according to some logging it passes it to weewx just fine, but for some reason weewx seems to be silently ignoring the data, it is never added to the database with no indication why in the logfile." What do you mean by this, or what are you expecting to see? If you are using hardware record generation (you must be if genArchiveRecords() is being called) then WeeWX will never save the loop packet data to the archive table in the database, it saves the archive record emitted by genArchiveRecords() to the archive table in the database. WeeWX only uses the loop packet data to update the daily summary tables. Did you look at the daily summary tables, are the highs and lows and timestamps being updated with loop data?

When you disable genArchiveRecords(), even though you have hardware record generation set, the call to genArchiveRecords() fails and WeeWX automatically falls back to software record generation where WeeWX synthesises an archive record and saves that archive record to the archive table in the database.

From your description above it is quite possible that WeeWX and your driver is working exactly as intended, it might not be, but I think you will need to look at bit deeper to show this.

Gary

[michael.k...@gmx.at]

Since the Weatherflow Stations don't provide hardware generated records as far as I know, there isn't much sense of having a genArchiveRecords() in the driver, or may one consider the REST service as "hardware generated records"?

@Jan-Jaap, just to reassure where you are heading to: the goal is to provide a driver for Weatherflow stations, that lets weewx generate archive records from loop data, when everything is up and running. On top of that, if weewx didn't run for whatever reason, and is started again, the driver should get the missing data from the weatherflow REST service for that particular station, and backfill the data into the weewx database, and then back to generating loop packets and let weewx do the stuff, right?

[gjr80]

The big plus for implementing genArchiveRecords() is that if genArchiveRecords() can provide historical archive records you have the ability to catch up if WeeWX looses its connection to the station. Without genArchiveRecords() catchup is not possible. If there is no ability to provide historical archive records and genArchiverRecords() is just synthesising an archive record from what is essentially loop data you are just duplicating what WeeWX already does. 

Gary

[Jan-Jaap van der Geer]

On Wednesday, February 10, 2021 at 7:30:27 AM UTC+1 gjr80 wrote:

It might be worth revising the operation of genLoopPackets() and genArchiveRecords(), in particular when running under hardware and software record generation.

<Snip very useful information how weewx works> thanks!

 

Back to your situation. When you have genArchiveRecords() enabled you say that "The driver sees the data and according to some logging it passes it to weewx just fine, but for some reason weewx seems to be silently ignoring the data, it is never added to the database with no indication why in the logfile." What do you mean by this, or what are you expecting to see? If you are using hardware record generation (you must be if genArchiveRecords() is being called) then WeeWX will never save the loop packet data to the archive table in the database, it saves the archive record emitted by genArchiveRecords() to the archive table in the database. WeeWX only uses the loop packet data to update the daily summary tables.

OK, so that makes more sense now. And it actually worked like this as well, the version on github now has code to NOT return records in genArchiveRecords() that have been fetched from loop data because I noticed it was using the archive versions of that data and it was those that ended up in the database.

This is not something I want as the data coming from the REST interface is rounded and make some of the graphs look very blocky (I've notified WeatherFlow of that and they said they will look into that, but I don't know when). That might not be a very good reason (at least in the long run) to prefer the loop data, but nonetheless that is the main reason why I want that data to be based on the loop data.

 

Did you look at the daily summary tables, are the highs and lows and timestamps being updated with loop data?

No, I have not looked at those, sorry.

When you disable genArchiveRecords(), even though you have hardware record generation set, the call to genArchiveRecords() fails and WeeWX automatically falls back to software record generation where WeeWX synthesises an archive record and saves that archive record to the archive table in the database.

How do I disable genArchiveRecords()? Returning no records apparently doesn't do the trick. Should I cast an exception or something?

From your description above it is quite possible that WeeWX and your driver is working exactly as intended, it might not be, but I think you will need to look at bit deeper to show this.

Yes, I think you might be right. My problem (as mentioned above) is that I'd rather not use the archive data as the loop data is of higher quality.

Is the loop data being fetched in a separate thread? Another approach might be to in the loop-data method create the record but don't return it (or actually return it as well, doesn't seem to matter...), but store it in the driver. When the genArchiveRecords() is called, return that data. But that does feel like a hackey solution. And if the loop data is being fetched from another thread, I might have to learn a bit more about thread-safety stuff in Python (total newby when it comes to Python...)

Thanks for your detailed answers, much appreciated!

Cheers,

Jan-Jaap

[Jan-Jaap van der Geer]

On Wednesday, February 10, 2021 at 8:17:00 AM UTC+1 michael.k...@gmx.at wrote:

Since the Weatherflow Stations don't provide hardware generated records as far as I know, there isn't much sense of having a genArchiveRecords() in the driver, or may one consider the REST service as "hardware generated records"?

@Jan-Jaap, just to reassure where you are heading to: the goal is to provide a driver for Weatherflow stations, that lets weewx generate archive records from loop data, when everything is up and running. On top of that, if weewx didn't run for whatever reason, and is started again, the driver should get the missing data from the weatherflow REST service for that particular station, and backfill the data into the weewx database, and then back to generating loop packets and let weewx do the stuff, right?

That is correct.

Cheers,

Jan-Jaap

[Jan-Jaap van der Geer]

On Wednesday, February 10, 2021 at 9:13:49 PM UTC+1 Jan-Jaap van der Geer wrote:

How do I disable genArchiveRecords()? Returning no records apparently doesn't do the trick. Should I cast an exception or something?

It seems throwing a NotImplementedError does the job nicely. A bit hackey, but it works :)

So I suppose if the provided timestamp is old I can fetch from the REST interface, otherwise I throw a NotImplementedError, that should in theory do the trick...

Cheers,
Jan-Jaap

[gjr80]

I think it might be worth taking a couple of steps back here and having a look at what you are trying to do, if not for your benefit then for mine. As I understand it you have started with a weatherflow driver that picks up UDP packets and emits that data in loop packets for WeeWX to ingest. This driver requires WeeWX to operate with software record generation as the driver only emits loop packets. You have taken that driver and extended it to pick up historical data via a REST interface. The aim of this is to give the driver the ability to emit historical archive records so that if WeeWX loses connection to the UDP packets WeeWX can catchup when connection is restored. When I became involved in the conversation you were working on implementing the genArchiveRecords() method in your driver, presumably as a path to being able to have WeeWX backfill/catchup if WeeWX loses the UDP connection. Do I have this correct?

I gave some earlier advice that in order to be able to catchup/backfill you need implement genArchiveRecords(), that is not strictly true. In order for WeeWX to be able to catchup the driver needs to have a genStartupRecords() method. It so happens that if you look at the base class of your driver, weewx.drivers.AbstractDevice, you will see that your driver will have inherited a genStartupRecords() method from the base class but it simply calls the genArchiveRecords() method. So by implementing your genArchiveRecords() method you have automatically given your driver a (functional) genStartupRecords() method. Nothing wrong with that but as you have found with the type of station the weatherflow is (there are no explicit archive records available, you need to construct them) you get some odd behaviour when genArchiveRecords() is implemented. What I suspect your really want is to implement genStartupRecords() and genLoopPackets() but NOT implement genArchiveRecords() and operate WeeWX with software record generation. To do this all you would do is move your genArchiveRecords() code to genStartupRecords() and remove the genArchiveRecords() class definition from your driver (just let it inherit the genArchiveRecords() method from the base class). This will result in the driver emitting loop packets (as the original driver does) and WeeWX accumulating these loop packets, synthesising archive records  and saving the archive records to the database archive table. No archive records are emitted by the driver during normal operation. If the UDP connection is lost and later restored then when WeeWX restarts it will call the genStartupRecords() method with the last known timestamp in the archive table and the genStartupRecords() method will then emit all archive records since this timestamp. WeeWX will store these archive records in the database archive table. Once the catchup is completed normal driver operation resumes and WeeWX continues processing loop packets.

Does that make sense?

Gary

[Jan-Jaap van der Geer]

On Thursday, February 11, 2021 at 4:02:30 AM UTC+1 gjr80 wrote:

I think it might be worth taking a couple of steps back here and having a look at what you are trying to do, if not for your benefit then for mine. As I understand it you have started with a weatherflow driver that picks up UDP packets and emits that data in loop packets for WeeWX to ingest. This driver requires WeeWX to operate with software record generation as the driver only emits loop packets. You have taken that driver and extended it to pick up historical data via a REST interface. The aim of this is to give the driver the ability to emit historical archive records so that if WeeWX loses connection to the UDP packets WeeWX can catchup when connection is restored. When I became involved in the conversation you were working on implementing the genArchiveRecords() method in your driver, presumably as a path to being able to have WeeWX backfill/catchup if WeeWX loses the UDP connection. Do I have this correct?

Yes, that is correct :) 

I gave some earlier advice that in order to be able to catchup/backfill you need implement genArchiveRecords(), that is not strictly true. In order for WeeWX to be able to catchup the driver needs to have a genStartupRecords() method. It so happens that if you look at the base class of your driver, weewx.drivers.AbstractDevice, you will see that your driver will have inherited a genStartupRecords() method from the base class but it simply calls the genArchiveRecords() method. So by implementing your genArchiveRecords() method you have automatically given your driver a (functional) genStartupRecords() method. Nothing wrong with that but as you have found with the type of station the weatherflow is (there are no explicit archive records available, you need to construct them) you get some odd behaviour when genArchiveRecords() is implemented. What I suspect your really want is to implement genStartupRecords() and genLoopPackets() but NOT implement genArchiveRecords() and operate WeeWX with software record generation. To do this all you would do is move your genArchiveRecords() code to genStartupRecords() and remove the genArchiveRecords() class definition from your driver (just let it inherit the genArchiveRecords() method from the base class). This will result in the driver emitting loop packets (as the original driver does) and WeeWX accumulating these loop packets, synthesising archive records  and saving the archive records to the database archive table. No archive records are emitted by the driver during normal operation. If the UDP connection is lost and later restored then when WeeWX restarts it will call the genStartupRecords() method with the last known timestamp in the archive table and the genStartupRecords() method will then emit all archive records since this timestamp. WeeWX will store these archive records in the database archive table. Once the catchup is completed normal driver operation resumes and WeeWX continues processing loop packets.

Brilliant! This sounds like exactly the thing I want! I'm going to play with this! :)

Thanks!

Cheers,

Jan-Jaap

[Jan-Jaap van der Geer]

On Thursday, February 11, 2021 at 9:09:13 PM UTC+1 Jan-Jaap van der Geer wrote:

Brilliant! This sounds like exactly the thing I want! I'm going to play with this! :)

And it seems to work perfectly! Thanks again!

Cheers,

Jan-Jaap