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

Refractor MagneticFieldSensor to extend abstract class #2

Draft
wants to merge 2 commits into
base: magnetic
Choose a base branch
from

Conversation

bartmathijssen
Copy link

This is a draft to improve the MagneticFieldSensor. I worked on extending the MagneticFieldSensor from the abstract class introduced by Ellen Spertus. This is a draft because there are still some issues. I would like your feedback on how to resolve these issues. The issue I am facing at the moment is that when using the MagneticFieldSensor, the application won't run. The issue probably lies within MultipleValueSensor class I introduced.

@barreeeiroo
Copy link
Member

I'll take a look into it; I haven't really had time to check how the new changes to sensors work

@barreeeiroo
Copy link
Member

barreeeiroo commented Jan 17, 2020

@bartmathijssen I've updated the branch, so maybe you would like to take a look. I merged all the ucr changes, so there might be some changes.
Can you check if something needs to be solved?

@bartmathijssen
Copy link
Author

@barreeeiroo I updated the branch. Unfortunately, the compiled application still won't run. There has to be something wrong with the MultipleValueSensor class, as sensors based on the SingleValueSensor class do work.

Copy link
Member

@conorshipp conorshipp left a comment

Choose a reason for hiding this comment

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

Can I ask why you've moved all of the properties from SingleValueSensor.java to SensorBase.java?

@bartmathijssen
Copy link
Author

bartmathijssen commented Jan 18, 2020

@conorshipp SensorBase is the base for the SingleValueSensor and MultipleValueSensor classes. As these classes have a lot in common, only the onSensorChanged method would be different.

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.

3 participants