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

sensors: split out battery_status to new module #13073

Merged
merged 9 commits into from
Oct 21, 2019
Merged

Conversation

dagar
Copy link
Member

@dagar dagar commented Oct 2, 2019

Quick PR to split out the analog battery handling from the sensors module.

  • add to each board
  • add to rcS
  • remove BOARD_NUMBER_BRICKS ifdefs and skip the module on boards instead
  • rename analog_battery_status?

@dagar dagar force-pushed the pr-battery_status branch from f84e62c to 7ea13bb Compare October 2, 2019 18:07
@dagar dagar force-pushed the pr-battery_status branch from cd8e592 to 59f7d3c Compare October 2, 2019 18:47
dakejahl
dakejahl previously approved these changes Oct 9, 2019
Copy link
Contributor

@dakejahl dakejahl left a comment

Choose a reason for hiding this comment

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

Tested successfully. I can't actually get off the ground cuz ESC issues, but everything else works as expected.

@dagar
Copy link
Member Author

dagar commented Oct 14, 2019

This is a simple incremental step to get the battery handling out of sensors pipeline and helps improve modularity as these power setups continue to get more diverse.

This is going to conflict with some of @ItsTimmy's open PRs, but I'll get them rebased. #12673 #12551

@dagar dagar marked this pull request as ready for review October 14, 2019 16:40
@dagar dagar changed the title [WIP] sensors: split out battery_status to new module sensors: split out battery_status to new module Oct 14, 2019
@dagar dagar force-pushed the pr-battery_status branch from 6d152d3 to eda74fa Compare October 20, 2019 18:48
@SalimTerryLi
Copy link
Contributor

Is it necessary to move the whole adc function out of DriverFramework? Currently differential pressure sensor and battery which rely on adc are all based on df.

@dagar
Copy link
Member Author

dagar commented Oct 21, 2019

Is it necessary to move the whole adc function out of DriverFramework? Currently differential pressure sensor and battery which rely on adc are all based on df.

Yes, but that part actually shouldn't be too difficult. Short term the DriverFramework usage can be replaced with CDev (for block device access) and the new PX4 work queue (for period scheduling).

In the ADC case I'd like to consider merging the frontends and only having to provide a few minor calls for ADC access. Then most platforms could share this common ADC frontend. https://github.com/PX4/Firmware/blob/master/src/drivers/adc/adc.cpp

@dagar dagar force-pushed the pr-battery_status branch from eda74fa to 1ca0031 Compare October 21, 2019 13:47
@dagar
Copy link
Member Author

dagar commented Oct 21, 2019

@SalimTerryLi just to be clear on my previous comment, it's necessary medium term to get everything moved out of DriverFramework for several other important reasons, but short term it's not necessary for the sake of analog battery and airspeed.

@dagar dagar merged commit 6a0f524 into master Oct 21, 2019
@dagar dagar deleted the pr-battery_status branch October 21, 2019 17:40
@SalimTerryLi
Copy link
Contributor

@dagar Got it. I would like to have a try :)

@julianoes
Copy link
Contributor

This is going to conflict with some of @ItsTimmy's open PRs, but I'll get them rebased. #12673 #12551

@dagar are you still planning on doing that? If so, when? This is a bit unfortunate because @ItsTimmy had his PR ready to be merged a while ago and we were waiting on the release to go out to then merge these things. We should coordinate these things better otherwise it can be frustrating.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants