New commits to lnetatmo.py break existing functionality

[baiti]

I have a working python script that uses lnetatmo.py successfully to query two netatmo stations. Up until (eab2766) it worked flawlessly. The referenced commit was done for: https://github.com/philippelt/netatmo-api-python/issues/50#issuecomment-1634493836.

Now with the current version: Update lnetatmo.py it is no longer working, it throws an exception during execution. There were more commits in between. I can't tell which commit really broke it. Here is the scenario (I am showing only the relevant code):

weatherData = lnetatmo.WeatherStationData(authorization)

for station in weatherData.stations:
    station_data = []
    module_data  = []
    station      = weatherData.stationById(station)
    pprint.pprint(station)
    station_name = station['station_name']

When I run this, I get:

None
Traceback (most recent call last):
  File "/home/baiti/newest_netatmo/./netatmo_influx.py", line 51, in <module>
    station_name = station['station_name']
                   ~~~~~~~^^^^^^^^^^^^^^^^
TypeError: 'NoneType' object is not subscriptable

I would conclude that weatherData.stationById(station) returns nothing. I would further conclude that the commits to this file after the one referenced above, are not just new functions, they break existing functionality.

[rvk01]

This is probably the script from https://github.com/florianbeer/grafana-netatmo then

For now you can change this line in netatmo_influx.py: station = weatherData.stationById(station) into this: station = weatherData.stationByName(station)

Not sure if it should be fixed here or in the grafana-netatmo repo.

[baiti]

This is probably the script from https://github.com/florianbeer/grafana-netatmo then

For now you can change this line in netatmo_influx.py: station = weatherData.stationById(station) into this: station = weatherData.stationByName(station)

I can confirm that this fix is indeed working. Thx much.

Not sure if it should be fixed here or in the grafana-netatmo repo.

is stationById() no longer supported by lnetatmp.py? In this case it would be grafana-netatmo that has to adapt to the change, however, it isn't obvious why stationById() can't be supported anymore. Can you shed some light into this?

[rvk01]

is stationById() no longer supported by lnetatmp.py?

stationById() is still supported in lnetatmo.py but the parameter for that is sid (so the actual Id, as it should).

netatmo_influx.py uses weatherData.stations and that (now?) contains the names of the stations, not the Ids. see lnetatmo.py: " self.stations = { d['station_name'] : d for d in self.rawData } "

You could do " for station in weatherData.stationsIds: " in netatmo_influx.py, and then stationById() would work. (because of " self.stationIds = { d['_id'] : d for d in self.rawData } "

Not sure who was responsible for this breaking change, lnetatmo or grafana-netatmo ;) I don't know if previously the self.stations contained the Ids or the Names.

[baiti]

Thanks for explaining. Hard to tell what would be the best way to deal with this. There are most likely more packages using lnetatmo and not just grafana-netatmo? So more than one place potentially needs to make changes to their sample code. I was just looking at the samples here in this repo and it seems to me that the following is the better approach anyway:

# For each station in the account
for station in weather.stations:

    print("\nSTATION : %s\n" % weather.stations[station]["station_name"])

If I got it right, that would eliminate the use of either ...ByName or ...ById entirely? And it would work with older and newer versions of lnetatmo.py ?

[rvk01]

Thanks for explaining. Hard to tell what would be the best way to deal with this. There are most likely more packages using lnetatmo and not just grafana-netatmo?

As far as I can see is Ids a new addition. Previously there was no stationIds array in WeatherStationData. So using stationById() by itself would have been the wrong thing to use anyway. There was only self.stations with the station names. So iterating through WeatherStationData.stations you would have always needed to call stationByName(). (not sure why there was a stationById() back then.)

# For each station in the account
for station in weather.stations:

    print("\nSTATION : %s\n" % weather.stations[station]["station_name"])

If I got it right, that would eliminate the use of either ...ByName or ...ById entirely? And it would work with older and newer versions of lnetatmo.py ?

Yes you can do it like that. But that's doing the same thing as stationByName(). And it would still break old code because it would need to be adjusted to use stations[station]["station_name"]) ;)

It's best then to either use

for station_id in weatherData.stationIds:
    station = weatherData.stationById(station_id)

or

for station_name in weatherData.stations:
    station = weatherData.stationByName(station_name)

(using station_name and station_id to remove the confusion about assigning the structure in the line below it to station.)

[baiti]

(using station_name and station_id to remove the confusion about assigning the structure in the line below it to station.)

Yes! not beeing fluid in python I stumbled over that too. Not even obvious that the assignment to station in the for station ... loop would work at all. I am glad that you're pointing it out.

[philippelt]

Hello, thanks for reporting the bug and your analysis.

It was in fact a bad behavior of the stationById function that was accepting in previous release a station name ! :smiley:

To avoid disrupting existing code, I reestablished this wrong behavior.