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

New 1-wire topic using aliases (MQTT) #590

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open

Conversation

krzbor
Copy link

@krzbor krzbor commented Nov 23, 2024

Adding a new MQTT topic: "1wire_alias". This topic allows publishing temperatures from "1-wire" sensors in the following format:
mqtt_topic_base/1wire_alias/[alias]
where [alias] is the alias assigned to the 1-wire sensor in HeishaMon.

This solution eliminates the issue of replacing a faulty sensor. After replacing the sensor, simply assign the same alias to the new one.

@krzbor
Copy link
Author

krzbor commented Nov 24, 2024

I deeply apologize for committing the main.yml file. I have restored its previous value, but please remove any requests related to main.yml.

Currently, 1-wire is read synchronously, which means the program is paused for up to 750ms. During this pause, a yield is called, ensuring that WiFi works correctly. However, other issues may arise, such as UART read buffer overflow.
I have modified the code so that everything operates asynchronously.
The previous solution to this problem contained a bug:
  if ((DALLASASYNC) && ((unsigned long)(millis() - dallasTimer) > ((1000 * dallasTimerWait) - 1000)) ) {
    DS18B20.requestTemperatures(); // get temperatures for next run 1 second before getting the temperatures (async)
  }
The author assumed the code would execute only once, while in reality, it was executed multiple times per second before the reading.
Asynchronous 1-wire reading
@IgorYbema
Copy link
Contributor

Idea is great but the implementation of the idea isn't what I would have done. Creating another main tree under the heatpump topic doesn't look right. But I can't think of something better right now.
The reason I added aliases is exactly for the same idea. But the problem is that I would like the automation backend code to be backwards and forwards compatible and not break on newly added info. That is why I initially added alias as a subtopic of the temp sensor.

But maybe I need to finally add auto-detect for homeassistant and follow that schematic for mqtt and let go of the old style.

For now keep this PR but don't commit yet.

@krzbor
Copy link
Author

krzbor commented Nov 30, 2024

That's exactly how I wrote it. It works both the old way and the new way. It's backward compatible.

@krzbor
Copy link
Author

krzbor commented Dec 1, 2024

@IgorYbema - have you analyzed the modified program code? The first level is the name contained in the variable "mqtt_topic_base". It must be derived from there to enable handling multiple heat pumps. The second level is "1wire_alias", which I created based on "1wire". The subsequent levels are defined by the user's alias.
At no point did I remove the original "1wire", which functions exactly as before. It also includes the subtopic "alias". My modification involved presenting the temperature in parallel under the new topic "1wire_alias". However, this topic is not always constructed because the following conditions must be met:

  • The alias cannot be empty,
  • The alias cannot start with "$",
  • There cannot be "/" at the beginning or the end,
  • The topic cannot be empty internally—"//" is prohibited,
  • The alias cannot contain the characters "+" and "#".
    As you can see, the new topic is added and published under strictly defined conditions that in no way disrupt the original functionality, which must remain intact, especially due to empty or incompatible aliases.
    My modification does not require any configuration. The client subscribes to specified topics and is unaffected by the additional "1wire_alias" topic, which presents the same value as "1wire".
    I do not consider presenting the same value in two different topics to be an error—as I mentioned, such an approach is necessary, and the old topic must remain.

@IgorYbema
Copy link
Contributor

Yes I did check and ofcourse very thankful for your addition. But for now I just dislike adding the same value to mqtt.

@krzbor
Copy link
Author

krzbor commented Dec 1, 2024

Duplication is a common solution when maintaining compatibility is necessary. In many cases, we have basic functions and their "_ex" counterparts, which are simply extensions.

Fortunately, the extension I implemented expands the program code only minimally, so there should be no issues maintaining the code in the future. Currently, the value is presented in the tree by the sensor's identifier. There is no straightforward way to read this value using an alias.

If you're concerned about the duplicate temperature publication, it could be made configurable in the general settings. However, in my opinion, that would unnecessarily complicate things—it would require explaining what such an option is for, and even then, someone configuring HeishaMon for the first time likely wouldn't understand it.

Another approach could involve adding an additional parameter when configuring an alias, such as "Publish as an MQTT topic." Alternatively, encoded aliases like MQTT_[alias] could be created, where the MQTT_ prefix would be stripped, and the resulting alias would define an MQTT topic. However, I think this would be over-engineering the solution.

In my opinion, the current approach is effective. It sends data about the sensor immediately upon physical connection, requiring no additional configuration.

The alias solution is intended for users who want "something more" and understand the problem.

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

Successfully merging this pull request may close these issues.

2 participants