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

Implemented power supply status estimation #173

Open
wants to merge 1 commit into
base: noetic-devel
Choose a base branch
from

Conversation

ahrastnik
Copy link

This adds the ability to estimate the charging status, which is necessary information for autonomous charging.

Implementation details along with simulation, research data / measurements and limitations can be found here.

@ahrastnik ahrastnik added the ready label Oct 9, 2023
@ahrastnik ahrastnik requested a review from mjstn October 9, 2023 13:28
src/motor_hardware.cc Outdated Show resolved Hide resolved
@@ -68,6 +69,8 @@ int32_t g_odomLeft = 0;
int32_t g_odomRight = 0;
int32_t g_odomEvent = 0;

// 2x6S(12 V) Batteries connected in series
const uint8_t SLA_AGM_CELLS = 6 * 2;
Copy link
Contributor

Choose a reason for hiding this comment

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

I suppose you could use this but any of our batteries alone at 12V is 'low'. I do see later you use 2.175 sort of number that would be 13V. A nominal 12V gel cell is near 12.6V actually.
I leave this up to you to think about and you can leave this if you like except the 2.175 factor should be a factor to come out to more like 12.6 but again batteries brand new fresh totally charted can read 13v so up to you.

Copy link
Author

Choose a reason for hiding this comment

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

If you are referring to SLA_AGM_CELLS, this number only indicates the number of cells in the used battery bank - and is not used directly anywhere in the code, but rather only to determine voltages levels with factors as seen below.

Perhaps my choice in variable names wasn't great, but these three constants below are used only to determine the charging state and I found that these factors work best for that purpose alone.

@mjstn
Copy link
Contributor

mjstn commented Oct 11, 2023

I would say this is ok as long as you convert the printf to be a ROS_INFO call because if you don't we don't see this message in the log. Now maybe that printf was debug code, up to you. If it is debug code then use of ROS_DEBUG would be used if you want that debug code to be seen in the future. keep in mind that if you hammer this routine all the time ROS_INFO is bad and so is printf because it will 'spam like crazy'. In the past I have used other more tricky things like have some static counter and only do the ROS_INFO every 5 seconds worth of time or something like that.

@ahrastnik
Copy link
Author

ahrastnik commented Oct 11, 2023 via email

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

Successfully merging this pull request may close these issues.

2 participants