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

fix(mqtt): Handle observations with underscores #139

Merged
merged 1 commit into from
Mar 25, 2023

Conversation

nifoc
Copy link
Contributor

@nifoc nifoc commented Mar 25, 2023

Some (extended) weewx observations already include an underscore in their name, which means that simply splitting on _ didn't work as expected for at least the following observations:

  • lightning_distance
  • lightning_disturber_count
  • lightning_energy
  • lightning_noise_count
  • lightning_strike_count
  • pm10_0
  • pm1_0
  • pm2_5

Doing this via a RegEx might not be the cleanest solution, but I feel like it beats hardcoding every affected observation.

Some (extended) weewx observations already include an underscore in
their name, which means that simply splitting on `_` didn't work as
expected for at least the following observations:

- `lightning_distance`
- `lightning_disturber_count`
- `lightning_energy`
- `lightning_noise_count`
- `lightning_strike_count`
- `pm10_0`
- `pm1_0`
- `pm2_5`
@Daveiano
Copy link
Owner

Thank you very much for catching this!

I assume you are able to compile the files by yourself so you are not dependent on a new release (and you tested the changes)?

Don't worry about the failing tests, they are failing because actions, triggered by outside persons do not have access to secrets (which are needed for the forecast tests - I think I need to split tests here, but that's another problem...)

@Daveiano Daveiano added the bug Something isn't working label Mar 25, 2023
@nifoc
Copy link
Contributor Author

nifoc commented Mar 25, 2023

I meant to mark this as a draft because I didn't test it yet - seems like I forgot to do that. Sorry!

Will test it soon and get back to you!

@nifoc nifoc force-pushed the fix/mqtt_underscore_observations branch from 8baee50 to c459316 Compare March 25, 2023 20:50
@nifoc
Copy link
Contributor Author

nifoc commented Mar 25, 2023

I can confirm that it works as expected and that I don't see any JS errors.

Please ignore the rest. Figured out that I also have to set Rounding in skin.conf

The only thing I did notice, though I'm fairly certain it's unrelated to this PR:

(Partial) Websocket frame

"pm10_0_microgram_per_meter_cubed": "8.895",
"pm1_0_microgram_per_meter_cubed": "2.720",
"pm2_5_microgram_per_meter_cubed": "5.220"

weewx.conf

# Shortened to just the essentials
[StdRESTful]
    [[MQTT]]
        [[[inputs]]]
            [[[[pm1_0]]]]
                format = %.3f
            [[[[pm2_5]]]]
                format = %.3f
            [[[[pm10_0]]]]
                format = %.3f

And the stat tile on the website only renders one decimal place instead of three.

@Daveiano
Copy link
Owner

Yes, this is looking good, thank you!

Doing this via a RegEx might not be the cleanest solution, but I feel like it beats hardcoding every affected observation.

I agree.

Yes the rounding needs to be set explicitly here. This is kind of inconsistent in weewx because if you use the tag

$current.outTemp

it will return the value with the correct rounding: '20,7°C' (with one decimal - like used in the stat tiles) but if you use the series tag/method (like I need for the graphs and diagrams) it will return the outTemp values with like 3 decimals (I don't remember the exact number) so you need to specify a rounding.

Thanks again for your contribution, this will be included in the next bugfix release.

@Daveiano Daveiano merged commit 32204d6 into Daveiano:3.x Mar 25, 2023
@Daveiano Daveiano added the MQTT label Jun 10, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working MQTT
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants