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

Feature/psm #108

Merged
merged 10 commits into from
Nov 12, 2023
Merged

Feature/psm #108

merged 10 commits into from
Nov 12, 2023

Conversation

RoseKapps
Copy link
Contributor

PSM finished and tested
the i2c need to be physically fixed
just need to add README file

@RoseKapps RoseKapps added the feature New feature or request label Nov 1, 2023
@RoseKapps RoseKapps self-assigned this Nov 1, 2023
Copy link
Contributor

@PizzaAllTheWay PizzaAllTheWay left a comment

Choose a reason for hiding this comment

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

Overal very good job Rose. Just need to dlete some stuff and add a README.md file and you are good ^-^

key Outdated Show resolved Hide resolved
key.pub Outdated Show resolved Hide resolved
mission/internal_status/test/test_copyright.py Outdated Show resolved Hide resolved
mission/internal_status/test/test_flake8.py Outdated Show resolved Hide resolved
mission/internal_status/test/test_pep257.py Outdated Show resolved Hide resolved
Comment on lines +30 to +49
def get_voltage(self):
# Sometimes an I/O timeout or error happens, it will run again when the error disappears
try:
system_voltage = (self.channel_voltage.convert_and_read() *
self.psm_to_battery_voltage)
return system_voltage

except IOError:
return

def get_current(self):
try:
system_current = (self.channel_current.convert_and_read() -
self.psm_to_battery_current_offset
) * self.psm_to_battery_current_scale_factor

return system_current

except IOError:
return
Copy link
Member

Choose a reason for hiding this comment

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

You could avoid a bit of copy paste / nesting here by adding a decorator, f.ex:

def io_error_safe_read(func):
    def wrapper(*args, **kwargs):
        try:
            return func(*args, **kwargs)
        except IOError:
            return
    return wrapper

then:

    @io_error_safe_read
    def get_voltage(self):
        system_voltage = (self.channel_voltage.convert_and_read() *
                          self.psm_to_battery_voltage)
        return system_voltage

    @io_error_safe_read
    def get_current(self):
        system_current = (self.channel_current.convert_and_read() - 
                          self.psm_to_battery_current_offset
                         ) * self.psm_to_battery_current_scale_factor
        return system_current

from std_msgs.msg import Float32


class MinimalPublisher(Node):
Copy link
Member

Choose a reason for hiding this comment

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

Would rename this to something a bit more descriptive 🐱

Copy link
Contributor

@PizzaAllTheWay PizzaAllTheWay left a comment

Choose a reason for hiding this comment

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

Verry well done.
Verry IMPORTANT that you delete the "key" and "key.gen" files as I can connect to you git account and push code without your concent and without knowing your password. I sugest you also dissable this key just in case and make a new one in private and write it down somewhere safe :)

If you want to further improve PSM system you can try and implement what Cristopher has commented. But that is ONLY if you feel confident that you know how to do those changes, otherwise no further improvements needed, this is a good code for me :)

key Outdated Show resolved Hide resolved
key.pub Outdated Show resolved Hide resolved
Copy link
Contributor

@PizzaAllTheWay PizzaAllTheWay left a comment

Choose a reason for hiding this comment

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

Verry nice jesjes

@PizzaAllTheWay PizzaAllTheWay merged commit 211d94a into development Nov 12, 2023
1 check failed
@PizzaAllTheWay PizzaAllTheWay deleted the feature/PSM branch November 12, 2023 13:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants