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

HasPosition.get_units() always returns None #70

Open
kadykov opened this issue Aug 29, 2023 · 2 comments
Open

HasPosition.get_units() always returns None #70

kadykov opened this issue Aug 29, 2023 · 2 comments

Comments

@kadykov
Copy link
Contributor

kadykov commented Aug 29, 2023

To reproduce:

In the configuration yml file:

[daemon_name]
port = 39001
units = "millimeter"

In the daemon class:

class DaemonName(HasPosition, IsDaemon):
    _kind = "DaemonName"

    def __init__(self, name: str, config: Dict[str, Any], config_filepath: Path):
        super().__init__(name, config, config_filepath)
        self.logger.debug("Getting units from get.units(): %s", self.get_units())
        self.logger.debug("Getting units from the config: %s", self._config["units"])

From the client side:

import yaqc

daemon = yaqc.Client(port=39001)
daemon.get_units()

I see that get_units() is covered by tests, but I cannot check it because on my machine most of the tests fail on from yaqd_core import testing.

@ksunden
Copy link
Member

ksunden commented Aug 29, 2023

This is an intentional choice because units as config is not general to all hardware, and thus is not included in the gas position trait.

Some hardware reports it's units via firmware, other hardware has no units at all, while we have standardized how clients ask for units, how the daemon knows its units is dependent on the individual case.

I would consider adding a separate trait only about units-as-config but I'm reticent to add config-only traits.

@kadykov
Copy link
Contributor Author

kadykov commented Aug 30, 2023

It makes sense. Thank you for your answer.
Do you know if this is mentioned somewhere in the documentation?

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 a pull request may close this issue.

2 participants