|
|
| Author |
Message |
pcleats
Joined: 12 Dec 2005
Posts: 35
|
|
Posted: 19 July 2006, 17:24 PM Post subject: A better way |
 |
|
The code I have listed below is used to check the status of the battery. It works ok. I am just wondering if there is a better way to do it. Maybe roll it into a function or something like that.
| Code: | ' definitions for measuring Battery Parameters
Private Const VoltagePin as Byte = 13
Private Const CurrentPin as Byte = 14
Dim Voltage As Single
Dim OutVolt as Single
Dim Current as Single
Dim OutCurrent as Single
' Get the voltage measurement
Private Sub ReadVoltage()
Call Getadc(VoltagePin, Voltage)
OutVolt = Voltage * 14.75
Call BatteryStatus
If ((Temp = 1) and (Outvolt > 11.70) _
and (OutVolt < 13.77)) then
Call ClearLine()
End If
call Delay(0.25)
End sub
' Get the current measurement
Private Sub ReadCurrent()
Call Getadc(CurrentPin, Current)
OutCurrent = Current * 2.188
call Delay(0.25)
End Sub
' Check the current status of the batteries
Private Sub BatteryStatus()
If Outvolt < 11.70 Then
call LowBattery
elseif OutVolt >13.77 then
call BatteryCharging
End If
End Sub
Private Sub LowBattery()
Call LCD_DisplayStrAt("Low Batt", 2, 1)
Temp = 1
End Sub
Private Sub BatteryCharging()
Call LCD_DisplayStrAt("Charging", 2, 1)
Temp = 1
End Sub
'Clear line 2 of the LCD display
Private Sub ClearLine()
Dim i as Byte
i = 10
Do While (i > 0)
Call LCD_DisplayStrAt(" ", 2, i)
i = i -1
Loop
Temp = 0
End Sub |
I know it looks ulgy, but any help cleaning it up would be GREAT!.
Thanks
Patrick
|
|
| Back to top |
|
 |
dkinzer
Site Admin
Joined: 03 Sep 2005
Posts: 2499
Location: Portland, OR
|
|
Posted: 19 July 2006, 19:15 PM Post subject: |
|
|
First off, note that I reformatted your post, using the code tags. This makes the code much easier to read because it presents it in monospace font and preserves whitespace indentation. You can do this in your posts in one of several ways. Perhaps the simplest way is to paste your code in the post, highlight the code and then click the "Code" button (below the Subject edit box). This action adds the words "code" and "/code" enclosed in square brackets at the beginning and end, respectively, of the highlighted text. You can also click the "Code" button first, paste your code and then click the "Code" button again. Finally, you can type the code tags yourself.
As to your request for suggestions, it is not clear exactly what your goal is. It is common to encapsulate functionality like this in a module (a separate .bas file). However, all of the routines that you have shown are private. A module must have at least 1 public entity in order to be useful.
Secondly, it looks like the 'temp' variable is being used as a flag to cause or suppress updating the display. If that is the case, surely a better name can be divined that is suggestive of its purpose. You might be able to restructure the code so that such a flag is not required at all. If it can be done without other undesirable side effects (e.g. difficult to understand, too slow, too large, etc.), it is advisable to do so.
Thirdly, it would be an improvement to replace the use of "magic numbers" like 2.188 and 11.70 with defined constants. If the constant names are suggestive of their role, it makes the code easier to read. Moreover, it makes the code easier to maintain, especially when the magic number is used in multiple places, e.g. your low/high battery voltage thresholds.
Unless your input signals for the current/voltage have external filtering (or even if you do), you might want to add a moving average calculation. One way to do this without much fuss is to use a queue to hold the readings.
|
|
| Back to top |
|
|
pcleats
Joined: 12 Dec 2005
Posts: 35
|
|
Posted: 19 July 2006, 21:32 PM Post subject: |
|
|
| dkinzer wrote: | | As to your request for suggestions, it is not clear exactly what your goal is. It is common to encapsulate functionality like this in a module (a separate .bas file). |
What I am trying to do is monitor the status of the batteries in the video distribution box that is placed 500 to 1000 ft away from my surveillance van. The lcd display is in the van and data is sent to it see attached schematic.
| dkinzer wrote: | However, all of the routines that you have shown are private. A module must have at least 1 public entity in order to be useful.
Secondly, it looks like the 'temp' variable is being used as a flag to cause or suppress updating the display. If that is the case, surely a better name can be divined that is suggestive of its purpose. |
Bad name choice for the variable this has been fixed. The flag is used to determine if the 2nd line of the display needs to be cleared.
| dkinzer wrote: | You might be able to restructure the code so that such a flag is not required at all. If it can be done without other undesirable side effects (e.g. difficult to understand, too slow, too large, etc.), it is advisable to do so.
Thirdly, it would be an improvement to replace the use of "magic numbers" like 2.188 and 11.70 with defined constants. If the constant names are suggestive of their role, it makes the code easier to read. Moreover, it makes the code easier to maintain, especially when the magic number is used in multiple places, e.g. your low/high battery voltage thresholds. |
I have changed them to defined constants. The numbers are used in conjunction with the voltage deviders I have to ensure that the voltage at the ZX-24 pins do not exceed 5 volts.
| dkinzer wrote: | | Unless your input signals for the current/voltage have external filtering (or even if you do), you might want to add a moving average calculation. One way to do this without much fuss is to use a queue to hold the readings. |
What I need is the actual voltage and current reading at any given moment at least for now, until I know how long the batteries will run the items connected to it. Also I don't know how to do a moving average calculation.
| Description: |
|
Download |
| Filename: |
Distribution box.jpg |
| Filesize: |
82.16 KB |
| Downloaded: |
2387 Time(s) |
|
|
| Back to top |
|
|
dkinzer
Site Admin
Joined: 03 Sep 2005
Posts: 2499
Location: Portland, OR
|
|
Posted: 19 July 2006, 22:14 PM Post subject: |
|
|
In your schematic, D2 is shown backwards, assuming that you want it to illuminate when the relay is activated to interrupt the power.
| Quote: | | I don't know how to do a moving average calculation. |
A moving average is simply an average over a set of values that changes over time. For example, you might average the last 10 readings taken. This would be a 10-point moving average.
To implement a moving average you need a way to store the last N readings, adding a new reading and removing the oldest reading on each iteration. You can do this with an array that holds N data points but you also need a way to add the new and drop the oldest one. The straightforward but inefficient way to do this is to move all of the existing entries to the next higher index and always add the newest reading at the beginning of the array. A more efficient but more complicated way is to maintain an index where the new entry should be added, wrapping it back to the beginning when it reaches the end.
You also need a counter to indicate how many entries are in the list. This helps in getting the moving average "primed". The first N-1 readings will result in a moving average over a smaller number of readings.
Since a queue already embodies the logic to wrap the insertion and deletion indices, it can be simpler to use than to write your own code. Although queues are probably most often used with Byte data, they work just as well with multi-byte data like Single and Integer values. One caveat, of course, is that GetQueueCount() always returns the number of bytes in the queue. If you're using the queue for multi-byte data you have to divide that return value by the data element size in order to determine how many elements are in the queue.
The code for my Reflow Toaster Oven Controller uses a queue to implement a moving average for the oven temperature readings.
|
|
| Back to top |
|
|
|
ZBasic Home
Forum Rules
Memberlist
Usergroups
RSS Feed
Site Search
Links
Download
Digests and Subscriptions
Profile
Log in to check your private messages
Log in
Register