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

Create a service for NKT fianium #290

Open
wants to merge 11 commits into
base: develop
Choose a base branch
from
Open

Create a service for NKT fianium #290

wants to merge 11 commits into from

Conversation

pbaudoz
Copy link
Collaborator

@pbaudoz pbaudoz commented Feb 24, 2025

This PR aims to setup a service for Fianium + Varia config.

Todo:

  • Adjust simulated service to follow changes in hardware service.
  • Update service docs for the NKT.

@ivalaginja ivalaginja assigned ivalaginja and unassigned ivalaginja Feb 24, 2025
@ehpor
Copy link
Collaborator

ehpor commented Feb 24, 2025

I feel like combining the Fanium with the current nkt_superk service should be doable. It'd just require changing an enum and setting the right values when creating the methods on the class. Right now, creating the functions is done as

get_base_temperature = read_register(registerReadS16, Evo.REG_BASE_TEMPERATURE, ratio=0.1)
get_supply_voltage = read_register(registerReadU16, Evo.REG_SUPPLY_VOLTAGE, ratio=0.001)
get_external_control_input = read_register(registerReadU16, Evo.REG_EXTERNAL_CONTROL_INPUT, ratio=0.001)

etc...

But you can do the same in the __init__(). Something like:

def __init__(self, ...):
    ... # Other setup code.
    
    connected_device = self.config['connected_device'].lower()
    if connected_device == 'evo':
        device = Evo
    elif connected_device == 'fanium':
        device = Fanium
    else:
        raise NotImplementedError(f'Connected device {connected_device} not implemented.')
    
    # Common functions between Evo and Fanium.
    self.get_emission = read_register(registerReadU8, device.REG_EMISSION, ratio=0.5)
    self.get_emission = read_register(registerReadU8, device.REG_EMISSION, ratio=0.5)
    
    if device is Evo:
        self.get_base_temperature = read_register(registerReadS16, Evo.REG_BASE_TEMPERATURE, ratio=0.1)
    
    if device is Fanium:
        ... # Fanium specific functions.

Does that work for you?

@ivalaginja ivalaginja added hardware Integrate new hardware collaborators Worked on by external collaborator. Might need some extra help on code integration. labels Feb 25, 2025
Copy link
Collaborator

@ivalaginja ivalaginja left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great so far! Left some comments for further steps. We can go through how to work through failed CI tests and how to adapt the simulated service later.

Comment on lines +52 to +54
#REG_BASE_TEMPERATURE = 0x17
#REG_SUPPLY_VOLTAGE = 0x1D
#REG_EXTERNAL_CONTROL_INPUT = 0x94
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you have double-checked that the Fianium doesn't have any of these, delete them all. Together with the "added" comments, since that is obvious from the code.

}

if self.device is Evo:
funcs['current_setpoint'] = self.monitor_func(self.current_setpoint, self.set_current_setpoint)
funcs['evo_status'] = self.update_func(self.update_evo_status)
Copy link
Collaborator

@ivalaginja ivalaginja Feb 26, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am pretty sure we'll need an update function for the Fianium as well. For that you will need to check what you can read from it with some sort of "get" functions I suppose, similar to the Evo.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't forget to delete this file!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
collaborators Worked on by external collaborator. Might need some extra help on code integration. hardware Integrate new hardware
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants