Skip to content

feat: Add generators module #435

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

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

gpranav162
Copy link

Closes #432

@senthurayyappan
Copy link
Member

Hi @gpranav162, can you please fix all mypy errors (which are mostly due to missing type annotations)?

@gpranav162
Copy link
Author

Hi @senthurayyappan, I've fixed all the mypy errors. Please let me know if any further changes are needed!

raise ValueError(f"Evaluation error: {e}") from e


class CustomGenerator(SignalGenerator):
Copy link
Member

Choose a reason for hiding this comment

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

Can we please move this CustomGenerator class to the signal_generators module?

from typing import Any, Optional, Union


class SignalGenerator(ABC):
Copy link
Member

Choose a reason for hiding this comment

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

can we follow the update(t: float) pattern?

We'd want SignalGenerators objects to be able to output values for a given value of time

from .base import SignalGenerator


class ConstantGenerator(SignalGenerator):
Copy link
Member

Choose a reason for hiding this comment

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

Can we modify this class to generate a step function instead? The StepGenerator's step time can be set to zero to output a constant value if the user wants a constant output from a signal generator.

I think generating constants this way is less useful

@senthurayyappan
Copy link
Member

Hey @gpranav162, great work! Can we modify the structure of signal generators to follow our update pattern so that they work well with other components? We want this module to be used for generating setpoints in the future. Here is an example use case (pseudo code):

from opensourceleg.actuators import DephyActpack
from opensourceleg.extras.generators import SineGenerator
from opensourceleg.utilities import SoftRealTimeLoop

if __name__ == "__main__":
    actpack = DephyActpack(...)
    sp = SineGenerator(...)
    clock = SoftRealTimeLoop(...)
    
    with actpack:
        actpack.set_control_mode(...)
        
        for t in clock:
            val = sp.update(t)
            actpack.set_motor_position(val)

@senthurayyappan
Copy link
Member

We could also support pre-generating outputs and have an iter pattern with a __next__ method within the SignalGenerator class.

@gpranav162
Copy link
Author

Hi @senthurayyappan, I've made the suggested changes:

  1. Moved CustomGenerator class to the signal_generators module
  2. Renamed ImpulseGenerator to StepGenerator (step_time set to zero outputs constant value) and removed the ConstantGenerator class
  3. Refactored the base class to use the update(t) method instead of __call__
  4. Added support for iteration over precomputed outputs with the __next__ method

Please let me know if there’s anything else you’d like me to update!

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.

Add signal generator module with support for built-in and custom waveforms
2 participants