Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

station registration silently fails if hardware_name is not a @property #936

Closed
vinceskahan opened this issue Feb 18, 2024 · 3 comments
Closed

Comments

@vinceskahan
Copy link
Contributor

Seen on the WeatherFlowUDP module - discussion at https://groups.google.com/g/weewx-user/c/JWTsu4s3TvY

The longstanding WeatherFlowUDP driver does not declare its hardware name as a property, which currently results in station registration silently failing for sites using that driver.

  • https://www.weewx.com/docs/5.0/custom/drivers/#implement-the-driver does not specify that declaring hardware name as a property is required, so my reading says the driver meets the documented requirements
  • and ideally restx.py should check its assembled string before trying to register, and not even try to register if it assembles a string that doesn't pass muster, logging that as well

I'd also suggest the 'implement the driver' section in the Customization Guide might be made a little clearer than 'look at these examples' for how to do a driver. While we can see @property throughout those example, that doesn't really explain the 'why' nor what is a hard requirement for the driver authors.

I am uncertain if Rich's suggestion of adding @property in the driver above def hardware_name(self) in this particular driver actually works or not. After doing so with a new bogus URL in weewx.conf and restarting weewx it seems like registration is still failing although the debug string looks like JSON to me. I can't decipher what it's looking for nor why it was considered a bad request.

Looking for Registry in the log with a little debugging added to restx.py returned:

2024-02-18T10:10:10.631712-08:00 raspberrypi weewxd[1270]: DEBUG weewx.engine: Loading service weewx.restx.StdStationRegistry
2024-02-18T10:10:10.688123-08:00 raspberrypi weewxd[1270]: INFO weewx.restx: StationRegistry _registry_dict: {'log_success': True, 'log_failure': True, 'station_url': 'https://www.example.com/no-station-here3', 'station_type': 'WeatherFlowUDP', 'description': 'WeatherflowUDP Simulator test station', 'latitude': 47.31, 'longitude': -122.36, 'station_model': 'WeatherFlow', 'config_path': '/home/pi/weewx-data/weewx.conf', 'entry_path': '/home/pi/weewx-venv/lib/python3.11/site-packages/weewxd.py', 'delay_post': 18}
2024-02-18T10:10:10.689000-08:00 raspberrypi weewxd[1270]: INFO weewx.restx: StationRegistry: Station will be registered.
2024-02-18T10:10:10.689333-08:00 raspberrypi weewxd[1270]: DEBUG weewx.engine: Finished loading service weewx.restx.StdStationRegistry
2024-02-18T10:15:15.723043-08:00 raspberrypi weewxd[1270]: DEBUG weewx.restx: StationRegistry: Delaying post by 18 seconds
2024-02-18T10:15:33.724950-08:00 raspberrypi weewxd[1270]: DEBUG weewx.restx: StationRegistry url: 'https://weewx.com/api/v2/stations/'
2024-02-18T10:15:34.049807-08:00 raspberrypi weewxd[1270]: DEBUG weewx.restx: StationRegistry: Failed upload attempt 1: HTTP Error 400: BAD REQUEST
2024-02-18T10:15:39.054559-08:00 raspberrypi weewxd[1270]: DEBUG weewx.restx: StationRegistry url: 'https://weewx.com/api/v2/stations/'
2024-02-18T10:15:39.332800-08:00 raspberrypi weewxd[1270]: DEBUG weewx.restx: StationRegistry: Failed upload attempt 2: HTTP Error 400: BAD REQUEST
2024-02-18T10:15:44.337202-08:00 raspberrypi weewxd[1270]: DEBUG weewx.restx: StationRegistry url: 'https://weewx.com/api/v2/stations/'
2024-02-18T10:15:44.610332-08:00 raspberrypi weewxd[1270]: DEBUG weewx.restx: StationRegistry: Failed upload attempt 3: HTTP Error 400: BAD REQUEST
2024-02-18T10:15:44.613726-08:00 raspberrypi weewxd[1270]: ERROR weewx.restx: StationRegistry: Failed to publish record 2024-02-18 10:15:00 PST (1708280100): Failed upload after 3 tries
2024-02-18T10:20:18.311790-08:00 raspberrypi weewxd[1270]: DEBUG weewx.restx: StationRegistry: wait interval (300 < 86400) has not passed for record 2024-02-18 10:20:00 PST (1708280400)

Hope this helps....

@tkeffer
Copy link
Contributor

tkeffer commented Feb 19, 2024

I've changed weereg to give a little better error message. Now it will give something like

2024-02-18 16:32:26 weewxd[62397]: DEBUG weewx.restx: StationRegistry url: 'https://weewx.com/api/v2/stations/'
2024-02-18 16:32:53 weewxd[62397]: ERROR weewx.restx: StationRegistry: Failed to publish record 2024-02-18 16:32:00 PST (1708302720): b'FAIL. https://example.com is not a serious station_url'

As you can see, the problem is that https://example.com is not considered to be a "serious" URL.

As for the "bound method" problem, the documentation clearly says that hardware_name should be an attribute, not a method. If the driver is implementing it as a method, then the results are undefined.

@tkeffer tkeffer closed this as completed Feb 19, 2024
@vinceskahan
Copy link
Contributor Author

I dunno, I was being completely serious. Just wanted to use a bogus URL to make 'anything' unique enough to trigger the registration attempt but I was trying to be mindful of your automation and server load so you didn't try to query a non-existent URL for the images for the map. Perhaps "not a valid url" might be better wording there.

I do see the words in the docs, but certainly didn't really understand the meaning of them. What had (has) me confused is why the entire rest of weewx seemed to be able to handle not having @property there. Still think restx.py should be doing some validation vs. unexpected input and not even trying if it's getting what it considers garbage-in.

Regardless - thanks for taking a look. I got the upstream driver author to merge a PR for that driver so the underlying driver thing is resolved too if folks update their driver to something restx.py is expecting.

@tkeffer
Copy link
Contributor

tkeffer commented Feb 19, 2024

I'm being serious, too. I use http://example.com all the time for testing. I know the server will reject it, so I don't have to worry about it contaminating the registration database. I can also expect a known response.

There's no end of things we could check. In this case, if the author uses a method instead of a property, Python does the right thing: it raises a TypeError. That's the way duck typing works.

tkeffer added a commit that referenced this issue Feb 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants