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

Add json output to commandline #26

Merged
merged 2 commits into from
Jul 14, 2024
Merged

Conversation

SmithChart
Copy link
Contributor

This is an alternate version to #24 .
I've taken @sante85 's branch tested it, fixed some bugs, and cleaned it up a bit. Thanks for the heavy lifting @sante85! :-)

I am not totally happy with the duplication in command_line.py that we get with this. But I am not sure if it is worth another level of abstraction either...

@rosswarren If you do not want to add this feature let me know. I would, at least, like to remove the duplicate output of the battery status registers (see 067e0d4).

sante85 and others added 2 commits April 20, 2024 21:17
This adds json output to the command line tool.

Co-authored-by: Chris Fiege <chris@tinyhost.de>
The list controller.battery_voltage_control_register_names
and the method controller.get_battery_voltage_control_registers()
are actually duplicates to the following methods.

While it is fine to have both ways of reading the
battery_voltage_control_registers from a library perspective (and we
definitely want to have this feature for writing), we do not need to
output all values two times on the command line.

Removing the variant with the least pretty formatting.
@rosswarren
Copy link
Owner

this looks good thank you 👍 I agree the duplication is OK for now, at least it is obvious from looking at the code what is happening

@rosswarren rosswarren merged commit a256644 into rosswarren:main Jul 14, 2024
@rosswarren rosswarren mentioned this pull request Jul 14, 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

Successfully merging this pull request may close these issues.

None yet

3 participants