Log in

View Full Version : New C18 3.0+ Compatible FRC Code


Pages : [1] 2

Kevin Watson
26-12-2007, 03:36
I've created C18 3.0+ compatible code and libraries for the IFI FRC robot controller. At this point the code is pretty solid and free of bugs. If interested, the code can be found here:

http://kevin.org/frc

Remember this is experimental code, so please do me (and everyone else who might use this code) a favor and let me know if you run into any bugs. Thanks.

1/6/08 Edit: You need MPLAB 7.21 or greater to use the 3.0+ compiler. You can download the latest version here:
http://www.microchip.com/stellent/idcplg?IdcService=SS_GET_PAGE&nodeId=1406&dDocName=en019469&part=SW007002

The student and upgrade versions of C18 3.10 (don't use 3.15) are here:
http://www.microchip.com/stellent/idcplg?IdcService=SS_GET_PAGE&nodeId=1406&dDocName=en010014&part=SW006011

-Kevin

Jon236
26-12-2007, 06:59
Kevin,

I like the layout....a lot cleaner than the present code template. Had a little problem compiling:
Executing: "C:\mcc18\bin\mcc18.exe" -p=18F8722 "ifi_frc.c" -fo="ifi_frc.o" -Ou- -Ot- -Ob- -Op- -Or- -Od- -Opa-
C:\dev\ifi_frc_beta\ifi_frc.c:249:Warning [2002] unknown pragma 'tmpdata'
C:\dev\ifi_frc_beta\ifi_frc.c:250:Error [1020] unexpected input following 'interrupt'
C:\dev\ifi_frc_beta\ifi_frc.c:357:Warning [2002] unknown pragma 'tmpdata'

Suggestions?

Joohoo
26-12-2007, 08:51
I do like the layout better than the code from years past. A little less cryptic in function definitions, and file names. Although I do not know what the Disable_spin(), Autonomous_Spin(), and Teleop_Spin() are and what they would be used for.
Are their equivalents be Process_Data_From_Local_IO() just split up into different places??

Also I do like the addition of the check of the state to reset the outputs. Shouldn't this stop situations where you can only get one autonomous run from each hard reset?

I cannot confirm or deny any bugs on a compile. I have neither mplab nor mcc18 and only have a mac ;)

Kevin Watson
26-12-2007, 10:45
Had a little problem compiling...It's not backward compatible with the 2.4 compiler. I'll create a 2.4 friendly version if people like the layout. The update to 3.1 is available on Microchip's website:

http://ww1.microchip.com/downloads/en/DeviceDoc/MPLAB-C18-Upgrade-doc-v3_10.exe

-Kevin

Kevin Watson
26-12-2007, 11:00
...I do not know what the Disable_spin(), Autonomous_Spin(), and Teleop_Spin() are and what they would be used for. Are their equivalents be Process_Data_From_Local_IO() just split up into different places?? Yes, you're correct.

...Also I do like the addition of the check of the state to reset the outputs. Shouldn't this stop situations where you can only get one autonomous run from each hard reset?Shouldn't be a problem as long as the controller is sent to the disabled state first and you reinitialize your autonomous code. Perhaps I could provide a flag to let you know to initialize your code?

-Kevin

bear24rw
26-12-2007, 14:03
Is there any advantage to using the 3.0+ compiler?

billbo911
26-12-2007, 14:27
Kevin,
I really like the structure of this code. It is MUCH cleaner and easier to follow. It will definitely help in teaching our newer programmers how to code specifically for FRC. I really like the use of the *_Spin functions. They make following and understanding the code and how it operates much easier, not only for new programmers, but also for relative Noobs to coding, like myself.

I see you are also taking advantage of the 3.0+ interrupt optimization capabilities. As we write ISR's will it be necessary to modify the way you are excluding the ".tmpdata""#pragma tmpdata low_isr_tmpdata
#pragma interruptlow Interrupt_Handler_Low nosave=section(".tmpdata")"

If I understood the included "C18 ISR.pdf", we should be fairly safe and not have to modify it at all, correct?



PS. I am really interested in making this code work with Vex if possible. I know this is a ways down the road, but it looks like it may just work. I'll keep you posted. I will wait to start down that road until the issues you pointed out are cleaned up first.

bagawk
26-12-2007, 14:46
Thanks Kevin! The code is much cleaner and better documented than what ifi gives. The interrupts setup will be a great help to a lot of teams I am sure.

BTW, I have an updated version of my makefile to build this code if anyone is interested.
http://team997.org/files/software/Makefile_ifi_frc

EHaskins
26-12-2007, 14:54
Overall I really like to layout of the code. It looks a lot more intuitive than the IFI stuff.

I really like that the serial port, interrupts, timers, and PWMs, are already intgrated. That should save some time during the season.

The only issue I've got is the x_Spin functions. It seems to me that most of the code that I would run there would be the same regardless of the current mode. I believe that I would modify it to call just one function, and use the mode flags to change the few things that are different.

EDIT: I haven't compiled it yet, since I don't want to mess up my main dev box. Tonight I'm going to setup a VM with c18 3.15. I'll post if I have any issues then.

billbo911
26-12-2007, 16:39
The more I read this code, the better I like it. I see you have included many of your "updated" functions as part of the base code. Thank you!!

I did note a couple things that may be a problem, but I am not certain because I am no expert when it comes to coding.

I noticed a couple "#include" statement for files that do not exist. For example, in "ifi_code.c" is a call to include "adc.h" and <delay.h>. I'm not clear on the differences when the "<xxxx>" is used, but it sure would be nice to have your "adc" code included.:)


Just my $.02

Kevin Sevcik
26-12-2007, 16:53
The layout looks a lot more straightforward than the standard IFI. To address the concerns of those who would rather have a single Spin function, you always have the option of defining your own Default_Spin() function and calling it from each of the other Spin functions. I really think that choice comes down to six of one and a half dozen of the other. If you'd rather have a single spin function and switch inside of it on flags, you're duplicating the mode determining logic already defined in the main loop for the privilege of shoving all your spinning code into one large function. I don't think saving a function call to a Default_Spin() routine is really worth that hassle.

lukevanoort
26-12-2007, 17:06
I'm sure I'm just being stupid or something like that, but whenever I try to open the workspace I get a "Unable to open the workspace because the format of the workspace file has changed" error followed by "A lock violation has occurred". I can open the individual *.c and *.h files just fine.

Anyway, I've only just started looking through this and it already looks awesome! I really like the timer setup and interrupt handling setup. Last year I did some interrupt and timer stuff and it could get rather confusing; this will be great for our new, inexperienced programmers to get into interrupts without quite as much trouble. It is almost like if you took the logical layout and ease of coding interrupts from WPIlib and combined them with the lower level control of IFI's code. Great work, Kevin!

EHaskins
26-12-2007, 20:29
I just setup a clean virtual machine with Windows XP Professional sp2. I ran Windows Update, installed MPLab 8.0, and installed C18 3.15 Student Edition.

Here are the errors I'm getting:
MPLAB C18 v3.15 (demo)
Copyright 1999-2005 Microchip Technology Inc.
Days remaining until demo becomes feature limited: 60
C:\Documents and Settings\Eric Haskins\Desktop\ifi_frc_beta\ifi_frc_beta\ifi_code .c:31:Error [1105] symbol 'ADC_VREFPLUS_VDD' has not been defined
C:\Documents and Settings\Eric Haskins\Desktop\ifi_frc_beta\ifi_frc_beta\ifi_code .c:31:Error [1105] symbol 'ADC_VREFMINUS_VSS' has not been defined
Halting build on first failure as requested.
BUILD FAILED: Wed Dec 26 19:25:06 2007

What did I forget?

EDIT: I moved it to a shallower directory, but it still returns that error.

EDIT: I uploaded a file with the output from the compiler.

billbo911
26-12-2007, 20:50
I just setup a clean virtual machine with Windows XP Professional sp2. I ran Windows Update, installed MPLab 8.0, and installed C18 3.15 Student Edition.

Here are the errors I'm getting:
MPLAB C18 v3.15 (demo)
Copyright 1999-2005 Microchip Technology Inc.
Days remaining until demo becomes feature limited: 60
C:\Documents and Settings\Eric Haskins\Desktop\ifi_frc_beta\ifi_frc_beta\ifi_code .c:31:Error [1105] symbol 'ADC_VREFPLUS_VDD' has not been defined
C:\Documents and Settings\Eric Haskins\Desktop\ifi_frc_beta\ifi_frc_beta\ifi_code .c:31:Error [1105] symbol 'ADC_VREFMINUS_VSS' has not been defined
Halting build on first failure as requested.
BUILD FAILED: Wed Dec 26 19:25:06 2007

What did I forget?

EDIT: I moved it to a shallower directory, but it still returns that error.

EDIT: I uploaded a file with the output from the compiler.


Honestly, I don't think you did anything wrong. I believe a file or two were left out of the zip. If you'll notice, at the top of ifi_code.c is a "#include adc.h", that file is not included in the .zip.

EHaskins
26-12-2007, 20:57
Honestly, I don't think you did anything wrong. I believe a file or two were left out of the zip. If you'll notice, at the top of ifi_code.c is a "#include adc.h", that file is not included in the .zip.

I believe that headers within <>s are found in "c:\mcc18\h" by default. If you look adc.h, delays.h, and stdio.h, are all located there.

Joe Ross
26-12-2007, 21:03
I just setup a clean virtual machine with Windows XP Professional sp2. I ran Windows Update, installed MPLab 8.0, and installed C18 3.15 Student Edition.

Here are the errors I'm getting:
MPLAB C18 v3.15 (demo)
Copyright 1999-2005 Microchip Technology Inc.
Days remaining until demo becomes feature limited: 60
C:\Documents and Settings\Eric Haskins\Desktop\ifi_frc_beta\ifi_frc_beta\ifi_code .c:31:Error [1105] symbol 'ADC_VREFPLUS_VDD' has not been defined
C:\Documents and Settings\Eric Haskins\Desktop\ifi_frc_beta\ifi_frc_beta\ifi_code .c:31:Error [1105] symbol 'ADC_VREFMINUS_VSS' has not been defined
Halting build on first failure as requested.
BUILD FAILED: Wed Dec 26 19:25:06 2007

adc.h should be included with the compiler. Are the include and library paths set? In the older MPLAB, the settings would be in project->build->project and would be:

C:MCC18\h for Include Path
C:MCC18\lib for Library Path

EHaskins
26-12-2007, 21:07
adc.h should be included with the compiler. Are the include and library paths set? In the older MPLAB, the settings would be in project->build->project and would be:

C:MCC18\h for Include Path
C:MCC18\lib for Library Path

Both of those are set properly. Also removing the reference to <adc.h> creates other errors including; (2058)Call of function without prototype and more (1105)s symbol has not been defined. That suggests to me that the issues is not in the MPLab configuration.

Joe Ross
26-12-2007, 21:15
Both of those are set properly. Also removing the reference to <adc.h> creates other errors including; (2058)Call of function without prototype and more (1105)s symbol has not been defined. That suggests to me that the issues is not in the MPLab configuration.

Those are exactly the errors you would expect if it can't find the include and library paths. Since you removed the adc.h include, it wouldn't have the prototype for the adc functions (first error). The symbols not defined error is because it can't find the the adc library that defines those symbols.

Kevin Watson
26-12-2007, 21:23
What did I forget?You forgot to read the readme.txt file :rolleyes:.

Microchip messed up the adc.h file in the latest release. Either fall back to 3.10 (using the handy link I provided above) or just comment out the offending code as it is not needed and I will be replacing it in the near future anyway.

-Kevin

Kevin Watson
26-12-2007, 21:26
If I understood the included "C18 ISR.pdf", we should be fairly safe and not have to modify it at all, correct?Yes, as long as you keep your code within the ISRs that I've provided in interrupts.c and timers.c, you should be fine.

-Kevin

EHaskins
26-12-2007, 21:29
You forgot to read the readme.txt file :rolleyes:.

Microchip messed up the adc.h file in the latest release. Either fall back to 3.10 (using the handy link I provided above) or just comment out the offending code as it is not needed and I will be replacing it in the near future anyway.

-Kevin

oops:o

Kevin Watson
26-12-2007, 21:33
I noticed a couple "#include" statement for files that do not exist. For example, in "ifi_code.c" is a call to include "adc.h" and <delay.h>. I'm not clear on the differences when the "<xxxx>" is used, but it sure would be nice to have your "adc" code included.:) Actually it's <adc.h> and <delays.h>. The <> tells the compiler to use it's own header files (usually in mcc18\h) instead of looking in the build directory. Once the code is stable and I have a sense that teams will want to use it, I'll build versions with support for some of my other code, like the ADC, gyro, encoder, etc.

-Kevin

Kevin Watson
26-12-2007, 21:36
I'm sure I'm just being stupid or something like that, but whenever I try to open the workspace I get a "Unable to open the workspace because the format of the workspace file has changed" error You'll need MPLAB 8.0 to open the project files. You can also just use the project wizard to create a new project.

-Kevin

billbo911
26-12-2007, 21:52
I believe that headers within <>s are found in "c:\mcc18\h" by default. If you look adc.h, delays.h, and stdio.h, are all located there.

So I have learned.:)
Well, as long as I keep learning, I don't mind hanging it out there. At least now I know more than I did this morning.:D

adc.h should be included with the compiler. Are the include and library paths set? In the older MPLAB, the settings would be in project->build->project and would be:

C:MCC18\h for Include Path
C:MCC18\lib for Library Path

Actually it's <adc.h> and <delays.h>. The <> tells the compiler to use it's own header files (usually in mcc18\h) instead of looking in the build directory. Once the code is stable and I have a sense that teams will want to use it, I'll build versions with support for some of my other code, like the ADC, gyro, encoder, etc.

-Kevin

That explains what I didn't understand. Thanks.

I am looking forward to the new builds that include your: ADC, gyro, encoder, PWM etc. I have learned a lot this summer by working with those versions from previous years and getting them to work with my Vex.
As of right now, I can fairly confidently say, we will use your code this year as long as the programmers don't decide to go with EasyC Pro. I need to let them make the choice.

ay2b
26-12-2007, 23:09
This is great! I like the new architecture for the main loop - it matches what I've been advocating to teams for years. One additional change I'd recommend is to have a "Teleop_Init()", "Autonomous_Init()" and "Disabled_Init()" that gets called as soon as the new mode is entered.

I've been going through and comparing the old to the new, but perhaps you can give a more detailed explanation of the changes. Some parts I've gone through line-by-line, other parts I haven't had the time to do so yet. Here's what I've identified:

In _startup() you have some assembly labeled "initialize all memory to zero", whereas in the old code, there was a function Clear_Memory(), which presumably does the same. All the other startup & initialization routines appear to be identical.

The main() function is now better organized, so that it calls a separate "slow loop" and a "fast loop" function for each of the three modes - autonomous, disabled and teleop. The "fast loop" functions are *_Spin(). I might suggest having a suffix for the "slow loop" functions too, so you have Autonomous_Slow_Loop() rather than just Autonomous(), but that's just my style.

The serial_ports.[ch] code appears to be the same.

The comments in pwm.[ch] lead me to believe it is all new, but I do not have an old version around to compare. Historically, I've ignored the CCP pins and spent my time working on other programming problems.

The timer code is basically the same code (because it only takes a few lines to set up a timer), but is a bit better organized. My main recommendation here would be to use the provide OpenTimer and WriteTimer macros, where applicable, rather than setting the magic variables directly. Even with all the comments, I think those macros make it easier for people new to the PIC to understand what's going on.

The interrupt code is also better organized than before, though I haven't yet gone through it line-by-line.

What sort of changes were needed to make it work with version 3.1? I have not yet read C18_ISR.pdf, which may answer this question for me. My impression from the comments here so far is that the main difference is in the way the interrupt handlers are set up, so I want to make sure I really take my time going through the new interrupt.[ch] files.

Overall, I'm happy with the changes made. The overall architecture now more closely matches the overall architecture I've been using for my team, which means that when I find myself at an event helping some random with their code, based on this, I will have an easier job.

Kevin Watson
27-12-2007, 01:12
I'd recommend is to have a "Teleop_Init()", "Autonomous_Init()" and "Disabled_Init()" that gets called as soon as the new mode is entered.This was my original plan but I wasn't sure if the added functions would just confuse people. I mentioned earlier that a good compromise might be to raise a flag once the mode changes. I'd very much like to know how people feel about this.

The timer code is basically the same code (because it only takes a few lines to set up a timer), but is a bit better organized. My main recommendation here would be to use the provide OpenTimer and WriteTimer macros, where applicable, rather than setting the magic variables directly. Even with all the comments, I think those macros make it easier for people new to the PIC to understand what's going on.

My philosophy is to include all the code so folks can see what's going on.

What sort of changes were needed to make it work with version 3.1? I have not yet read C18_ISR.pdf, which may answer this question for me. My impression from the comments here so far is that the main difference is in the way the interrupt handlers are set up, so I want to make sure I really take my time going through the new interrupt.[ch] files.
Most of the work went into writing the interface library (ifi_frc_xxxx.lib) that's compatable with the new COFF file format used by the 3.0 compiler. I was actually ready to release the libraries last February, but ran into a nasty bug that kept the project on the back burner until a few weeks ago. I found the bug a few days ago.

-Kevin

Mark McLeod
27-12-2007, 09:05
I mentioned earlier that a good compromise might be to raise a flag once the mode changes. I'd very much like to know how people feel about this.
I know what you mean about producing too many functions, especially in a single file. I have found it can muddle things for new programmers. In this case I think it can be handled with a flag in a couple of lines. Then we can teach the newbies how to make a function out of it.

EHaskins: The student version 3.15 can also be used if you fix the 3.15 version of the adc.h system file. There is a missing "#endif" at line 536. It does a series of PIC architecture checks and they forgot to close one conditional #if, thereby wiping out all the subsequent statements.

(I'm traveling in Va this week, so I won't be much help. I touch base (CD) when I can, which is actually more often than I expected to.)

billbo911
28-12-2007, 02:01
OK, another noob question.
I installed a clean version of MPLAB the "upgraded" it with the file from the link Kevin provided.
I can't get it to open Kevin's ifi_frc.mcp. So, after using the Project wizard to create a new project, it will compile correctly without errors. Well, that's partially true, I had to comment out a printf statement in user_code.c.
Now the output window keeps spitting this out:
"Error: Bad magic number in COFF file "C:\TEMP\C18 3.0beta\ifi_frc_beta\ifi_code.o".
So, what is that telling me? What is a "magic number"?

Guy Davidson
28-12-2007, 02:22
Definitely very cool. Finally we're up to the times and there will be no need to dig up one of the old kickoff CDs every time we want or need to be able to compile code on a new computer. I'll definitely check it out soon.

Tom Bottiglieri
28-12-2007, 02:58
I can't decide on whether or not I will split off the teleop, autonomous, and disable functions to their own files. I feel like user_code.c will be a bit cluttered and may be overwhelming for new student programmers.

Thoughts? Kevin, any reason you did not pursue something like this?

ay2b
28-12-2007, 03:44
I can't decide on whether or not I will split off the teleop, autonomous, and disable functions to their own files. I feel like user_code.c will be a bit cluttered and may be overwhelming for new student programmers.

Personally, I advocate doing this. I think keeping the teleop and autonomous sections separate is beneficial. If you have multiple programmers, it makes it easier for different people to work on different sections.

Additionally, I strongly advocate that everyone add a "robot.c/robot.h" that has all the robot functionality. This should be the only place that directly sets any PWM value. It would have functions like robot_drive(speed, turn_rate, gear) and robot_set_arm_position(height). These functions would be called from both the teleop and autonomous functions. Then, when you change your robot, swap motors around, change a pneumatic gear shifter to a servo gear shifter, bend a pin on the RC, etc, you only have one location where you need to make code changes.

Basically the robot.c/h file presents the robot conceptually to the rest of the code. The autonomous and teleop functions can deal with the logic, like "the robot drives forward X distance, and then turns 90 degrees", and the robot.c/h functions can translate that into "set PWM01=255, and PWM02=0".

Kevin Watson
28-12-2007, 13:39
I can't decide on whether or not I will split off the teleop, autonomous, and disable functions to their own files. I feel like user_code.c will be a bit cluttered and may be overwhelming for new student programmers.

Thoughts? Kevin, any reason you did not pursue something like this?Yes, this was considered, but I have a very small bias toward keeping the number of source files down to a minimum. Since most new teams probably won't touch the Autonomous( ) or Disabled( ) functions, adding four more source files would make the learning curve a little more steep. With that said, if we can reach some kind of consensus on this or any other aspect of the code, I'd be more than happy to modify the code to meet the needs of the community (which is one of the reasons I started this conversation).

-Kevin

Tom Bottiglieri
28-12-2007, 14:00
Yes, this was considered, but I have a very small bias toward keeping the number of source files down to a minimum. Since most new teams probably won't touch the Autonomous( ) or Disabled( ) functions, adding four more source files would make the learning curve a little more steep. With that said, if we can reach some kind of consensus on this or any other aspect of the code, I'd be more than happy to modify the code to meet the needs of the community (which is one of the reasons I started this conversation).
-Kevin
What is your stance on pre compiling your drivers and back end code into a library? The source would be available to read, but you could cut out some of the mess in the source tree.

Although, on second thought, this may be a pain to maintain.

billbo911
28-12-2007, 14:07
OK, another noob question.
I installed a clean version of MPLAB the "upgraded" it with the file from the link Kevin provided.
I can't get it to open Kevin's ifi_frc.mcp. So, after using the Project wizard to create a new project, it will compile correctly without errors. Well, that's partially true, I had to comment out a printf statement in user_code.c.
Now the output window keeps spitting this out:
"Error: Bad magic number in COFF file "C:\TEMP\C18 3.0beta\ifi_frc_beta\ifi_code.o".
So, what is that telling me? What is a "magic number"?

Just a bump. Still looking for the answer.

Kevin Watson
28-12-2007, 14:24
Just a bump. Still looking for the answer.What version of MPLAB are you using? If it's anything less than 7.4 (I think), it won't work with the new compiler.

-Kevin

Kevin Watson
28-12-2007, 14:38
What is your stance on pre compiling your drivers and back end code into a library? The source would be available to read, but you could cut out some of the mess in the source tree.

Although, on second thought, this may be a pain to maintain.No, it's a good idea and one I'm entertaining (e.g., I already have a version of ifi_frc_xxxx.lib with the serial port code included).

My interest right now is to make sure that the CD programmer community is okay with the architecture and, hopefully, will spend some time finding any bugs I've introduced. Once that's done, I'll have a look at simplifying the source tree.

Speaking of bugs, does anyone have any code which will consistantly cause the 8.3 volt problem? I have a theory I'd like to test out, but haven't been able to reproduce the bug.

-Kevin

billbo911
28-12-2007, 15:02
What version of MPLAB are you using? If it's anything less than 7.4 (I think), it won't work with the new compiler.

-Kevin

From my original post,

OK, another noob question.
I installed a clean version of MPLAB then "upgraded" it with the file from the link Kevin provided.I can't get it to open Kevin's ifi_frc.mcp....Now the output window keeps spitting this out:
"Error: Bad magic number in COFF file "C:\TEMP\C18 3.0beta\ifi_frc_beta\ifi_code.o".
So, what is that telling me? What is a "magic number"?


So, I thought it was 8.10, but now that I think about it, I originally installed 7.4 and the upgrade is only for C18, so I might still be at MPLAB 7.4, and C18 is 3.1. I'll verify when I return home this evening. Thanks Kevin! :)

Kevin Watson
28-12-2007, 15:54
So, I thought it was 8.10, but now that I think about it, I originally installed 7.4 and the upgrade is only for C18, so I might still be at MPLAB 7.4, and C18 is 3.1. I'll verify when I return home this evening. Thanks Kevin! :)From the C18 3.10 release notes: "MPLAB C18 v3.00 and later will not be compatible with versions of the MPLAB IDE prior to v7.21".

-Kevin

billbo911
28-12-2007, 20:10
Yep, you nailed it Kevin.

I had installed MPLAB 7.20, added C18 2.x then upgraded C18 to 3.10.

So, now that I have reinstalled MPLAB 8.0 with C18 3.10, it works like a champ! It loads and builds your project first time, every time.

Now I'm off to try and see if I can actually do something with it. :ahh:

Binome
28-12-2007, 20:31
Thanks immensely. I use C18 3.10 all the time, and its a pain to have to maintain two seperate development toolchains for general purpose and FRC specific code. It's great not having to go searching for that ancient C-BOT compiler disk whenever I need to set up a box for FRC code, when I can get the most recent MPLAB IDE and C18 from the microchip website.

eugenebrooks
28-12-2007, 23:05
Speaking of bugs, does anyone have any code which will consistantly cause the 8.3 volt problem? I have a theory I'd like to test out, but haven't been able to reproduce the bug.

-Kevin

We don't have a program that could run on a bare controller, but I believe that we still have our 2006 robot fully together (currently with a 2005 controller) and could put the 2006 controller and program back in it without much effort. I would not say that it "consistently" hit the the 8.3 volt bug, but it most certainly hit the problem frequently. If you have a patch in mind we would be willing to try it out during the build period. It would be really nice to fix that bug. We have sworn off using interrupts (in our own code) on the 8722 based controllers until the problem is addressed.

Eugene

Kevin Watson
29-12-2007, 00:57
We don't have a program that could run on a bare controller, but I believe that we still have our 2006 robot fully together (currently with a 2005 controller) and could put the 2006 controller and program back in it without much effort. I would not say that it "consistently" hit the the 8.3 volt bug, but it most certainly hit the problem frequently. If you have a patch in mind we would be willing to try it out during the build period. It would be really nice to fix that bug. We have sworn off using interrupts (in our own code) on the 8722 based controllers until the problem is addressed.

EugeneThanks. I'd be happy if I could just get a .map or .lst file of a build that exhibited the problem.

-Kevin

eugenebrooks
29-12-2007, 01:33
Thanks. I'd be happy if I could just get a .map or .lst file of a build that exhibited the problem.

-Kevin

Kevin,

I have sent you a web link to download a zipped example via a private message.

Eugene

Kevin Watson
29-12-2007, 04:15
Kevin,

I have sent you a web link to download a zipped example via a private message.

EugeneI got the code. Thanks.

-Kevin

Lafleur
29-12-2007, 18:03
I noticed that you did not add the:

#pragma tmpdata low_isr_tmpdata

#pragma tmpdata

in the serial interrupts handlers?? but you did so in the timer.c and interrupts.c handlers??

is this an oversight??

thanks...

ayeckley
29-12-2007, 20:20
I wonder if Kevin's release of this updated code (albeit a beta version) implies that the IFI default code will also be heavily revised for 2008. If nothing else, I think it strongly implies that the canonical IFI RC/OI package will be in the KOP this year, even if it is the last time.

Alex

Kevin Watson
29-12-2007, 20:39
...is this an oversight??Yes, it certainly is. Thanks for catching it.

-Kevin

Kevin Watson
29-12-2007, 21:02
I wonder if Kevin's release of this updated code (albeit a beta version) implies that the IFI default code will also be heavily revised for 2008. If nothing else, I think it strongly implies that the canonical IFI RC/OI package will be in the KOP this year, even if it is the last time.

AlexOr, given that Dave Lavery has the PsyOps and misinformation dials cranked up to eleven, you might consider that I could possibly be doing his dirty work by misleading everyone. Heck, for all I know, you could be working for Dave too and are planting counter-misinformation :D.

-Kevin

Guy Davidson
29-12-2007, 21:58
Kevin,

I noticed this file does not include any encoder-related files. Are you planning to add support to encoders to this code?

Thanks.

lukevanoort
29-12-2007, 22:23
Kevin,

I noticed this file does not include any encoder-related files. Are you planning to add support to encoders to this code?

Thanks.
It should be pretty easy to write your own encoder code with this architecture's method of handling interrupts. It would only take a few if statements and a couple of very short functions to read from a quadrature encoder. Much of the trouble with writing encoder code is configuring interrupts, and Kevin has already done that here.

You could basically copy and paste the code from the encoder driver's ISRs into the ISRs in this version (or put them in another file and call them from the ISRs, which would probably be more readable) and add the Get_Encoder_x_Count() function. Actually, if you just didn't call the initialization functions, you could probably use the current encoder code verbatim (well, for encoders one and two anyway, for 3-6 you'd probably just want to reuse the code from encoder 1-2 instead of the encoder driver's functions for encoders 3-6). I would say that it would probably be less than 20mins to code.

gnormhurst
29-12-2007, 22:48
Great design. Just a word about words.

The *_Spin() functions are great -- it's just that in my little robot namespace world "spin" refers to when the robot turns around its axis. (We set up the joystick so one of the buttons calls the spin() routine that makes the robot turn on a dime.) When I saw "spin" that's what I thought it meant; I had to study the code to see what it really meant.

Is "spin" so standard in the embedded world that I should invent a new term for "turning on a dime"? (Diming? What if you only turn 90 degrees? Is that Quartering?) Or can we name the _Spin() functions something else? _WhileWeAreWaitingForNewData()? _StudyHall()? _Loitering()?

Sorry for the bad jokes, but I'm seriously concerned about overloading the term "spin"...

It just me?

Kevin Watson
29-12-2007, 23:21
Is "spin" so standard in the embedded world?In computer science a "spin loop" is a way to waste time while waiting for something to happen.

-Kevin

Kevin Watson
29-12-2007, 23:26
Are you planning to add support to encoders to this code?If I get the sense teams will use the new code, I'll create versions with support for some sensors.

-Kevin

Lafleur
29-12-2007, 23:58
I noted that when I look at the .map file there is no reference to the low.isr.tmpdata in the map file ?? why is this?? I wanted to see how much space the compiler is giving to this... .tmpdata is there...

thanks...

Kevin Watson
30-12-2007, 00:29
I noted that when I look at the .map file there is no reference to the low.isr.tmpdata in the map file ?? why is this?? I wanted to see how much space the compiler is giving to this... .tmpdata is there...

thanks...The linker won't create the section if no temp storage is needed. Try enabling one of the interrupts and put this code in the interrupt service routine:

int a = 2;
int b = 3;
int c = 4;
int d;

d = (a + b + c) - (c + a);

Compile and then look at the .map file.

-Kevin

ayeckley
30-12-2007, 09:13
misinformation dials cranked up to eleven

In case we need that extra push over the cliff right about now :)

Alex

Jon236
30-12-2007, 09:53
Kevin,

If you're looking for a head count for your template, count the TechnoTicks (236) in!

Guy Davidson
31-12-2007, 11:51
You could also probably count us (8) in too.

I have a somewhat related question that stems from encoders. How fast to digital inputs update? Is it the 26.6ms loop? Faster? I'm asking because I'm wondering about updating the encoder B channel. If it doesn't update fast enough, aren't we risking using an old value and so getting the wrong direction?

Alan Anderson
31-12-2007, 12:37
How fast to digital inputs update?

Treat the digital I/O as instantaneous. It's faster than the execution speed of the program instructions. What you read from an input pin is the state of the pin at that moment. What you write to an output pin takes effect immediately. There are measurable (but trivial) delays, but unlike PWM and relay outputs, there's no interprocessor software communication in the way to slow things down.

Guy Davidson
31-12-2007, 13:00
Treat the digital I/O as instantaneous. It's faster than the execution speed of the program instructions. What you read from an input pin is the state of the pin at that moment. What you write to an output pin takes effect immediately. There are measurable (but trivial) delays, but unlike PWM and relay outputs, there's no interprocessor software communication in the way to slow things down.

Very cool. Just what I was hoping to hear. Hopefully I'll have this new code driving a robot and looking at encoders before the week is over, and I'll be happy to give you (Kevin) any feedback or bugs I find.

EHaskins
31-12-2007, 13:41
We(1103) willl be using this code. So far it looks good.

Lafleur
31-12-2007, 14:23
The linker won't create the section if no temp storage is needed. Try enabling one of the interrupts and put this code in the interrupt service routine:

int a = 2;
int b = 3;
int c = 4;
int d;

d = (a + b + c) - (c + a);

Compile and then look at the .map file.

-Kevin


Well in ver 3.10 of the compiler the .map file will NOT show an entry if there is no storage needed, but 3.15 will show a null entry. (as it should)

PhilBot
31-12-2007, 17:31
Hi Kevin.

Your new structure makes a lot more sense. I never liked the way the old code fell into the "autonomous" pit and never came out.

I installed MPLAB8 and upgraded to cc18 3.1. Other than the typical include/lib path issues asociated with any upgrade, your new code built just fine.

My own personal preference is to use more, smaller files, so my vote would be to split the auto and disabled modes out into sepparate files. I beleive that this would actually make it easier for teams not addressing these modes because they wouldn't have to keep scrolling arround the empty functions.

If you are lucky to have more than one programmer on the team, it does enable the "Autonomous" mode programmer to work somewhat independantly of the "teleop" programmer. Last year I also had c&h files for each of the four subsystems: UserIF, Drive, Manipulator and Vision, and assigned each subsystem to a student. It kept them out of each other's hair, and made it easier to see what had changed from version to version. Not much downside to extra files.

To Fill up the new files a bit, I would also vote for including the init_ functions. So, each of the mode files would have an init, spin and run function.

I also agree with the recomendation for teams to have a robot.h and robot.c file. It's the first thing I add to the standard ifi code. In my case this is where I put all my #defines for translating the cryptic I/O port names to robot functions, and code for enforcing various robot safety limits.

Team 1629 will definately be using your new code so I'm eager to see you fold all your other cool functions into this structure.

Phil.
A happy camper:D

Spencer E.
31-12-2007, 18:16
Wow Kevin! I love it already. Everything is so well organized and everything is incorporated. This will be a really good tool for rookies as well as veteran teams :). I will definitely be using this on our 2008 robot :D

PhilBot
31-12-2007, 18:18
Just noticed this:

In your pwm_readme.txt file, you still reference Process_Data_From_Master_uP()

I guess this would now be: Teleop() and Autonomous(), although I see you make the call after these functions.

Phil.

Kevin Watson
31-12-2007, 19:32
My own personal preference is to use more, smaller files, so my vote would be to split the auto and disabled modes out into sepparate files. I beleive that this would actually make it easier for teams not addressing these modes because they wouldn't have to keep scrolling arround the empty functions...

...To Fill up the new files a bit, I would also vote for including the init_ functions. So, each of the mode files would have an init, spin and run function.Okay, I've implemented most of this. Is there really a need for a Disabled_Init( ) function?

-Kevin

billbo911
31-12-2007, 20:31
Okay, I've implemented most of this. Is there really a need for a Disabled_Init( ) function?

-Kevin


Good question!

One way to look at it is:
During Competition, the systems boot up in Disabled mode. It wouldn't be a bad place to do some of the system initializations, Pre-calibrations and OI input gathering. Gyros would love the fact that the bot is not moving and compressors aren't running.

On the other hand, it is a bit redundant considering the Initialize () routine already exists in user_code.c.

Honestly, I say, give it the boot. (Yes, pun intended:ahh: )

Kevin Watson
31-12-2007, 21:48
Here is a snapshot of the current build:

http://kevin.org/frc/ifi_frc_beta_2.zip

-Kevin

Guy Davidson
31-12-2007, 23:32
Okay, I've implemented most of this. Is there really a need for a Disabled_Init( ) function?

-Kevin

I would say it isn't. To my understanding (and the way I'd use it) the whole point of the disabled function is to initialize variables and do some setup. Hence, as far as I'm concerned, there's no need for an initializing function to the initializing function.

ay2b
01-01-2008, 00:10
Okay, I've implemented most of this. Is there really a need for a Disabled_Init( ) function?

Checking our code for the last few years, we've had a Disabled_Init(), but mostly just used it for debugging. I think it's worth including, just for completeness. Similarly, each year we've had a Disabled_Spin() function , but it's always been empty. I think it's worth having all three functions (run, spin, init) in the framework for each mode.


actually we called ours Disabled_Fast_Loop(); and the run function was Disabled_Slow_Loop(), but I think I like _Spin() and _Run() better.

Kevin Sevcik
01-01-2008, 00:20
I would say it isn't. To my understanding (and the way I'd use it) the whole point of the disabled function is to initialize variables and do some setup. Hence, as far as I'm concerned, there's no need for an initializing function to the initializing function.Well as I understand it, you're still getting valid data from the OI in Disabled mode and teams have used this fact to select auto modes before matches start. So presumably, you'd want a Disabled_Init() to re-initialize this variable to a default value if you had to do a restart. I can see uses for it, but mostly I think it should be left in just for consistency between the different modes.

Kevin Watson
01-01-2008, 02:05
Well as I understand it, you're still getting valid data from the OI in Disabled mode and teams have used this fact to select auto modes before matches start. So presumably, you'd want a Disabled_Init() to re-initialize this variable to a default value if you had to do a restart. I can see uses for it, but mostly I think it should be left in just for consistency between the different modes.Okay, it's easy enough to do, but disabled mode is nominally called three times each round (before autonomous, between autonomous and teleop, and after teleop). Should Disabled_Init( ) be called each time, or only the first time? If only the first time, under what conditions would the default_init_flag get reset?

-Kevin

billbo911
01-01-2008, 03:59
Should Disabled_Init( ) be called each time, or only the first time? If only the first time, under what conditions would the default_init_flag get reset?

-Kevin

It should be called only the first time and then reset on power cycle or on RC reset.
Just my opinion.

lukevanoort
01-01-2008, 09:46
It should be called only the first time and then reset on power cycle or on RC reset.
Just my opinion.
I would agree, but I am a person who likes to keep options open. With that goal in mind, I think it would be ideal to put the code that calls Disabled_Init() in an #ifdef conditional so a programmer can just quickly define/undefine something to change the behavior.

PhilBot
01-01-2008, 10:03
Okay, it's easy enough to do, but disabled mode is nominally called three times each round (before autonomous, between autonomous and teleop, and after teleop). Should Disabled_Init( ) be called each time, or only the first time? If only the first time, under what conditions would the default_init_flag get reset?
-Kevin

In answer to your initial question about the need for a Disabled_Init() function... we also used this to trigger calibrating the Gyro. The actual callibration only occured if we stayed in disabled mode for several seconds.

We put in the delay because we didn't know if the system transitioned through Disabled between auto and teleop. Based on your post, I'm glad we took that precaution.

We also did it for code symetry more than anything else.... Yes, I also indent all my "=" signs the same amount. Just call me anal.

I think the Disabled_Init() is most usefull once before the match begins, so that would mean that you wouldn't really want it called between Auto and teleop.

I thought that it might also be good to call it on the transition from Teleop back into Disabled (to indicate that there might have been a field reset situation after a bad start) but the bot is in an unknown state at that time anyway (maybe on it's side) so you wouldn't necessarily want to make any callibration assumptions.

In the end I guess this function would only be called after reset, so it is a bit redundant with Initialize(). Although, if the function is resetting variables that only get used in disabled mode, then my anal side says that that's where they should be declared and initialized :) This is also what we did with our UserIF, Drive, Manipulator and VisionSystem variables. Declared and initialized in subsystem modules.

Thanks for even considering it :rolleyes:
Phil.

PhilBot
01-01-2008, 11:26
Here is a snapshot of the current build:

http://kevin.org/frc/ifi_frc_beta_2.zip

-Kevin

That's looking excellent.

trivial note: I don't know whether you update your comments along the way, or once the build has stabilized, but I just wanted to mention that the ifi_frc.c file still has all the old reference to user_code.c in it's comments.

Another thing I realized when I loaded this project was that your development environment and mine don't match, so it caused a hickup that may trip up first time users (me two years back:)

You have the CC18 environment loaded under the MPLAB program folder, wheras mine is loaded under the c:/ root. I have no idea if this was a decision of mine, or the compiler default (i think so), or a consequence of me simply "upgrading" last year's install, but it means that when I load your project, two things happen:

1) MPLabs prompts me as to whether I want to keep my old CC18 compiler and Linker folders or use the ones in the project (I say keep the old one)

2) I get a linker error because MPLAB can't find the clib library. I need to go to Build-Options/Directories/Library, and set the path from c:/program files/mplab/cc18/lib to c:/cc18/lib

My only reason for bringing this up is to determine what you, and the likely "critical mass" of users will be using. If CC18 should be loaded under MPLAB then I'll move mine. If this is contrary to the install default, then a "pre-install" readme would be a good idea to prevent (minimize) the initial confusion of new users.

Thanks for all the time you put in.

Phil.

ay2b
01-01-2008, 14:45
Okay, it's easy enough to do, but disabled mode is nominally called three times each round (before autonomous, between autonomous and teleop, and after teleop). Should Disabled_Init( ) be called each time, or only the first time? If only the first time, under what conditions would the default_init_flag get reset?

I believe it should be called every time that the robot enters into disabled mode.

Guy Davidson
02-01-2008, 18:42
You could basically copy and paste the code from the encoder driver's ISRs into the ISRs in this version (or put them in another file and call them from the ISRs, which would probably be more readable) and add the Get_Encoder_x_Count() function. Actually, if you just didn't call the initialization functions, you could probably use the current encoder code verbatim (well, for encoders one and two anyway, for 3-6 you'd probably just want to reuse the code from encoder 1-2 instead of the encoder driver's functions for encoders 3-6). I would say that it would probably be less than 20mins to code.

Thanks. For now, I commented out the code for encoders 3-6, and I use the code for 1 and 2 by having the Int_1_Handler() in interrupts.c call on Encoder_1_Int_Handler() in encoder.c. This sounds like it should work, and I'll post here if it works or if it fails and I can't figure it out by myself.

How about gyros? I noticed that your gyro code from last year uses both gyro.c/h and adc.c/h. I copied both of thme, and I've been fumbling for a while with integrating them and timers.c. I have not been able to do that yet, generating a red-light-of-doom in the process. Any ideas or advice?

EDIT: Kevin, I believe I may have found a problem. Interrupts.h uses #define ENABLE_INT_# (for # being 1,2,...), while ifi_frc.c checks #ifdef ENABLE_INTR_# when checking for the ISR. That is an oversight, right? I searched the rest of the files and could not find another reference to ENABLE_INTR_#.

billbo911
02-01-2008, 20:23
Okay, it's easy enough to do, but disabled mode is nominally called three times each round (before autonomous, between autonomous and teleop, and after teleop). Should Disabled_Init( ) be called each time, or only the first time? If only the first time, under what conditions would the default_init_flag get reset?

-Kevin

Another thought popped into my head about Disabled_Init(). The three times a robot gets disabled is true in most years games, but some years it happens a lot more. Remember 2005, every time the human player stepped off the enable mat to load a Tetra on a robot, that robot was disabled. So, with this in mind, the function would definitely need a flag that would only allow it to initialize on reboot or power up.

Kevin Watson
02-01-2008, 21:23
Thanks. For now, I commented out the code for encoders 3-6, and I use the code for 1 and 2 by having the Int_1_Handler() in interrupts.c call on Encoder_1_Int_Handler() in encoder.c. This sounds like it should work, and I'll post here if it works or if it fails and I can't figure it out by myself.

How about gyros? I noticed that your gyro code from last year uses both gyro.c/h and adc.c/h. I copied both of thme, and I've been fumbling for a while with integrating them and timers.c. I have not been able to do that yet, generating a red-light-of-doom in the process. Any ideas or advice?

EDIT: Kevin, I believe I may have found a problem. Interrupts.h uses #define ENABLE_INT_# (for # being 1,2,...), while ifi_frc.c checks #ifdef ENABLE_INTR_# when checking for the ISR. That is an oversight, right? I searched the rest of the files and could not find another reference to ENABLE_INTR_#.You might want to wait on the encoders as I have a scheme to make the encoder ISRs more efficient. I'll be coding it up sometime in the next few days.

I'll also port the ADC code too at some point (I need to find a better way of installing ISRs).

I'll look into the INTR vs. INT issue this evening. It's probably just some knuckle-headedness on my part. Thanks for catching it.

-Kevin

Mike Mahar
03-01-2008, 15:03
How do you get a version 8 MPLAB? The disks that we've received from FIRST were all version 7.20. IFIRobotics' web site still claims that they only work with 7.20. I didn't notice the MPLAB requirement and now I have a hybred system that won't compile my 2.4 code nore will it open Kevin's new workspace.

Tom Bottiglieri
03-01-2008, 15:14
How do you get a version 8 MPLAB? The disks that we've received from FIRST were all version 7.20. IFIRobotics' web site still claims that they only work with 7.20. I didn't notice the MPLAB requirement and now I have a hybred system that won't compile my 2.4 code nore will it open Kevin's new workspace.
www.microchip.com
It's free

EHaskins
03-01-2008, 15:15
How do you get a version 8 MPLAB? The disks that we've received from FIRST were all version 7.20. IFIRobotics' web site still claims that they only work with 7.20. I didn't notice the MPLAB requirement and now I have a hybred system that won't compile my 2.4 code nore will it open Kevin's new workspace.

MPLab v8 can be downloaded directly from Microchip.com (http://microchip.com/stellent/idcplg?IdcService=SS_GET_PAGE&nodeId=1406&dDocName=en019469&part=SW007002).

I have had no issues running c18 v2.4 or v3.10 with MPLab v8.

Your 2.4 code will not compile with c18 v3, and the MPLab v8 workspaces will not open in older versions of MPLab. If you want to be able to compile your 2.4 code, you'll need to copy the mcc18 folder from a computer with 2.4 installed, and change the header, linker, and library serch folders of you 2.4 projects to point to the 2.4 folders.

Code Monkey
03-01-2008, 15:34
Last year we got an updated adc code which only used 1 interrupt per sample, instead of 2. Is this already implemented in the new adc code? Also last year we had to use a new processor .h file for the encoders. I see the encoder work is in progress, so this will also disappear, I hope?

Isn't it new to be able to program for the disabled mode? Is there a hint there?

EHaskins
03-01-2008, 15:41
Isn't it new to be able to program for the disabled mode? Is there a hint there?
Previously the same code would be run in the robot was Disabled or Enabled. The outputs were just disabled.

lukevanoort
03-01-2008, 16:02
Previously the same code would be run in the robot was Disabled or Enabled. The outputs were just disabled.
Actually, you could always determine if the robot was disabled or enabled, the default IFI code just didn't make use of it and neither did most teams' custom code.

My team, and probably several others (I know Chief Delphi does it or at least did it at one point), implemented mode determination to help prevent integrator wind up in PID loops.

EHaskins
03-01-2008, 16:04
Actually, you could always determine if the robot was disabled or enabled, the default IFI code just didn't make use of it and neither did most teams' custom code.

My team, and probably several others (I know Chief Delphi does it or at least did it at one point), implemented mode determination to help prevent integrator wind up in PID loops.

That's the only reason I ever did it.

Code Monkey
03-01-2008, 16:25
Looking back at other posts, I see that now. It is nice for newer teams to see that they could initialize the gyro while siting and waiting for the match to begin. Now that is explicit. This newer code does look straight forward.

I won't get a chance to actually download this into a CD prior to kick off, but will spend my lunches studying it.

Thanks to all of you already solving problems!

Capt. Quirk
03-01-2008, 18:08
I see there is a .lkr file 18f8520, does that mean I can play with it on a VEX bot?

EHaskins
03-01-2008, 18:09
I see there is a .lkr file 18f8520, does that mean I can play with it on a VEX bot?

No, you also need a version of the library for vex.

Guy Davidson
03-01-2008, 19:28
Kevin,

While we're looking at the new code, what is the difference between interrupts 1,2 and 3-6? I read the encoders readme, and I understand how they differ. However, I haven't seen any documentation about the interrupts themselves. Could you please explain me how (if it all) they differ?

Alan Anderson
03-01-2008, 21:33
Kevin,

While we're looking at the new code, what is the difference between interrupts 1,2 and 3-6? I read the encoders readme, and I understand how they differ. However, I haven't seen any documentation about the interrupts themselves. Could you please explain me how (if it all) they differ?

Digital inputs 1 and 2 each go to separate interrupt circuits. They can be configured to cause an interrupt on either a low-to-high or high-to-low transition of the input. The interrupt service routine can inspect individual flags to determine which input caused the interrupt.

Digital inputs 3-6 all go together to a third interrupt circuit, and automatically cause an interrupt on any input transition. There's only one flag to say that "something changed" on those pins. In order to determine which of the four inputs is responsible, the software has to do its own bookkeeping to compare the previous input state against the present one.

That's a simple explanation. There's more detail in the PIC manual if you want it.

Guy Davidson
03-01-2008, 21:45
Digital inputs 3-6 all go together to a third interrupt circuit, and automatically cause an interrupt on any input transition. There's only one flag to say that "something changed" on those pins. In order to determine which of the four inputs is responsible, the software has to do its own bookkeeping to compare the previous input state against the present one.


This makes sense. Thank you. I'll try using some of them soon, hopefully the ISRs in Kevin's code work as-is and then the difference won't really matter to me.

Kevin Watson
04-01-2008, 03:31
Last year we got an updated adc code which only used 1 interrupt per sample, instead of 2. Is this already implemented in the new adc code?

This evening I integrated a modified version of the ADC and gyro code into the new RC code and it seems to be very solid (while doing other things, including a bunch of printf()s, I have the ADC sampling at 6400Hz with no problems). The new ADC code uses the 8722s acquisition delay feature, which allows me to run it at one interrupt per sample, instead of two. I should be able to release it tomorrow.

Also last year we had to use a new processor .h file for the encoders. I see the encoder work is in progress, so this will also disappear, I hope?This is next on my list.

-Kevin

Mike Mahar
04-01-2008, 09:53
Thanks for the link to Microchip. I went there three times and couldn't find it. The page looked like a sales page so I didn't scroll down and see the download links. Oh well. I've got it now.

Ed Sparks
04-01-2008, 11:00
..... I should be able to release it tomorrow.

-Kevin


Thank you Kevin for all of your hard work! I like what I see so far and I can assure you that team 34 will put this to good use.

Lafleur
04-01-2008, 11:11
Kevin:

Here are some change that I made to your serial routine to set the baudrate correctly for any processor speed.

in serial_port.c

#ifdef ENABLE_SERIAL_PORT_ONE
void Init_Serial_Port_One(void)
{
// Start by initializing the serial port with code
// common to receive and transmit functions
// SPBRG = BAUD_38400 ; // baud rate generator register [251]
SPBRG = SPBRG_VAL_1; // defined in hardware.h
//
#ifdef USART1_USE_BRGH_LOW
TXSTAbits.BRGH = 0; // high baud rate select bit (asynchronous mode only) [248]
#else // 0: low speed
TXSTAbits.BRGH = 1; // high baud rate select bit (asynchronous mode only) [248] // 1: high speed
#endif // 1: high speed


.................................................. .................................................. .................................................. ......


#ifdef ENABLE_SERIAL_PORT_TWO
void Init_Serial_Port_Two(void)
{
// Start by initializing the serial port with code
// common to receive and transmit functions
// SPBRG2 = BAUD_9600; // baud rate generator register [251]
SPBRG2 = SPBRG_VAL_2; // defined in hardware.h
//
#ifdef USART2_USE_BRGH_LOW
TXSTA2bits.BRGH = 0; // high baud rate select bit (asynchronous mode only) [248]
#else // 0: low speed
TXSTA2bits.BRGH = 1; // high baud rate select bit (asynchronous mode only) [248] // 1: high speed
#endif // 1: high speed





------------------------------------------------------------------------------------------------------------------------------------------------------------


in the hardware set up routines...




// Define system xtal/clock frequency
//
#define CLOCK_FREQ (40000000) // Frequency in Hz

#if defined(__18CXX) // All PIC18 processors
#include <p18cxxx.h>
#define INSTR_FREQ (CLOCK_FREQ/4) // PIC18 clock divider
#else
#error Unknown processor or processor not defined
#endif


// Set up serial port(s) baud rate
//
#define BAUD_RATE_1 (38400) // Serial port 1, bps
#define BAUD_RATE_2 (38400) // Serial Port 2, bps

// set as needed...
//#define USART1_USE_BRGH_LOW // for most lower baud rates
//#define USART2_USE_BRGH_LOW // for most lower baud rates

#if defined(USART1_USE_BRGH_LOW)
#define SPBRG_VAL_1 ( ((CLOCK_FREQ/BAUD_RATE_1)/64) - 1)
#else
#define SPBRG_VAL_1 ( ((CLOCK_FREQ/BAUD_RATE_1)/16) - 1)
#endif

#if defined(USART2_USE_BRGH_LOW)
#define SPBRG_VAL_2 ( ((CLOCK_FREQ/BAUD_RATE_2)/64) - 1)
#else
#define SPBRG_VAL_2 ( ((CLOCK_FREQ/BAUD_RATE_2)/16) - 1)
#endif

#if ((SPBRG_VAL_1 > 255) || (SPBRG_VAL_2 > 255))
#error "Calculated SPBRG value is out of range for current CLOCK_FREQ."
#endif

Code Monkey
04-01-2008, 14:19
To be clear, without Kevin this would be miserable. Team 1622 is massively
in your debt. Thank you.

For those who have not already installed the MCC upgrade, make sure you put it in the same directory with the upgraded MPLAB 8.0. Otherwise the linker can't find the libraries. I know by the trying it the wrong way.

comphappy
04-01-2008, 19:23
Rookie team 2605 will be using your code, it is structured in a way that makes more sense, I have not gone through it all the way yet, but so far so good. BTW is there a better search tool in mplab than the just straight basic find? For my embedded systems work I am always using grep and regexps, so far mplab has been really disappointing.

B.Johnston
04-01-2008, 21:16
Comphappy,

The answer maybe Yes!

Try under the Project Menu > Find in Project Files.

Once you enter your keyword you'll be able to see the results in the output window under the find in files tab.

Double click on whatever you want to see and a new window will open with a pointer to your chosen element.

I hope this answers your question, as I'm not really sure as to what grep would deliver as a result.

comphappy
04-01-2008, 22:01
Thanks that is what I was looking for, still not very good, but better then what I had found. Grep is extreamly flexible long as you are willing to spend the time to learn how to use it correctly.

Kevin Watson
04-01-2008, 22:22
Here's a snapshot of the latest build:

http://kevin.org/frc/ifi_frc_beta_3.zip
http://kevin.org/frc/ifi_frc_gyro_beta.zip (with integrated ADC and Gyro code)

-Kevin

Guy Davidson
04-01-2008, 22:33
Kevin,

Is there any change log? So we can keep track of what's bew in beta 3? Other than, of course, the ADC and gyro in that version.

Thanks.

jtdowney
05-01-2008, 00:03
I know MPLAB can generate a makefile for you but those always seem messy so I wrote up a quick one to use for myself with Kevin's new code. Figured I'd share it here in case anyone else finds it useful.

# Makefile for Kevin Watson's (http://kevin.org) C18 3.0 compatible FRC code
CC = C:\\mcc18\\bin\\mcc18.exe
CFLAGS = -p=18F8722 -k -mL -Ou- -Ot- -Ob- -Op- -Or- -Od- -Opa-
LD = C:\\mcc18\\bin\\mplink.exe
LDFLAGS = 18f8722.lkr
LIBS = ifi_frc_8722.lib
LIBPATH = C:\\mcc18\\lib
MAP = ifi_frc.map
RM = del

SOURCES = autonomous.c disabled.c ifi_code.c ifi_frc.c interrupts.c pwm.c \
serial_ports.c teleop.c timers.c
OBJECTS = $(SOURCES:.c=.o)
EXECUTABLE = ifi_frc

all: $(SOURCES) $(EXECUTABLE)

$(EXECUTABLE): $(OBJECTS)
$(LD) /l"$(LIBPATH)" $(LDFLAGS) $(OBJECTS) $(LIBS) /m"$(MAP)" /w /o"$(EXECUTABLE).cof"

.c.o:
$(CC) $< -fo="$@" $(CFLAGS)

clean:
$(RM) $(OBJECTS) $(EXECUTABLE).cof $(EXECUTABLE).hex $(MAP)

sparrowkc
05-01-2008, 18:57
Forgive my noobiness, but is that makefile what I need to develop on ubuntu? Set up c18 in wine, put the file in the directory with the code and do a make? Will that give me a loadable hex file? There was another makefile posted earlier that looked like it was made for Linux, should I use that?

emmell
05-01-2008, 19:01
Here's a snapshot of the latest build:

http://kevin.org/frc/ifi_frc_beta_3.zip
http://kevin.org/frc/ifi_frc_gyro_beta.zip (with integrated ADC and Gyro code)

-Kevin

Kevin:

Thank you. Thank you. Thank you. Team 832 (as well as other Georgia teams I help with their programming) will definitely make use of your new version.

All of the FIRST community is in your debt for helping all of the teams.

Mannie Lowe

jtdowney
05-01-2008, 19:06
Forgive my noobiness, but is that makefile what I need to develop on ubuntu? Set up c18 in wine, put the file in the directory with the code and do a make? Will that give me a loadable hex file? There was another makefile posted earlier that looked like it was made for Linux, should I use that?

I personally am not using it on Linux but if you change the path of the CC and LD to be the path to wine + the path to the executable, I would imagine it'd work for you no problem. Then you'd just change the value of RM to actually rm.

sparrowkc
05-01-2008, 19:17
Great! Thanks! I hate working in a VM.

CPress
05-01-2008, 21:47
Thanks Kevin.

Being a new(ish) c programmer, it really helps with all of the useful comments and such. The code is very clean and organized which is always easier to read.

thanks again.

1jbinder
05-01-2008, 23:35
Hi,
I'm a freshman in my team, but i have a lot of C experience and was able to decrypt lasts years code. First of all I really like this layout much better. Is there bugs in the code to the point where i should not use part of it for the code this year. Also i know this requires a 3.0+ complier and i was wondering what complier the FIRST FRC controller is meant to work with. I might have this all totally wrong with complier versions because I am new to the complier and also to FIRST.
Thanks
1jbinder
Team 852

jtdowney
05-01-2008, 23:41
Hi,
I'm a freshman in my team, but i have a lot of C experience and was able to decrypt lasts years code. First of all I really like this layout much better. Is there bugs in the code to the point where i should not use part of it for the code this year. Also i know this requires a 3.0+ complier and i was wondering what complier the FIRST FRC controller is meant to work with. I might have this all totally wrong with complier versions because I am new to the complier and also to FIRST.
Thanks
1jbinder
Team 852

There certainly may be bugs but the chances of them not being found before ship is low (Remember with enough eyes, all bugs are shallow). I would think it is perfectly safe to use Kevin's new code on this years RC. The default code from IFI is years past is meant to compile with C18 2.4. I haven't seen this years default code but I suspect that is still the case.

Kevin Watson
05-01-2008, 23:48
...i know this requires a 3.0+ complier and i was wondering what complier the FIRST FRC controller is meant to work with.The officially supplied version is 2.4, but a free upgrade to 3.10 (don't use 3.15) is available on Microchip's website (http://ww1.microchip.com/downloads/en/DeviceDoc/MPLAB-C18-Upgrade-doc-v3_10.exe). I haven't tested it yet, but the freebie student version will probably work just fine. For those who want to stick with 2.4, I just built a version that will work with the older compiler. After I do a bit of testing, I'll post a link to it.

-Kevin

Kevin Watson
06-01-2008, 01:23
Here's my first cut at C18 2.4 compatible code:

http://kevin.org/frc/ifi_frc_24_beta.zip
http://kevin.org/frc/ifi_frc_gyro_24_beta.zip (with ADC and gyro code)

Remember this is experimental code, so please do me (and everyone else who might use this code) a favor and let me know if you run into any bugs. Thanks.

-Kevin

EHaskins
06-01-2008, 01:56
Here's my first cut at C18 2.4 compatible code:

http://kevin.org/frc/ifi_frc_24_beta.zip
http://kevin.org/frc/ifi_frc_gyro_24_beta.zip (with ADC and gyro code)

Remember this is experimental code, so please do me (and everyone else who might use this code) a favor and let me know if you run into any bugs. Thanks.

-Kevin

Can I expect ADC and Gyro code for 3+ to be released soon(within the next week), or should I use the 2.4 version if I need that functionallity?

Thanks,
Eric

Kevin Watson
06-01-2008, 02:07
Can I expect ADC and Gyro code for 3+ to be released soon(within the next week), or should I use the 2.4 version if I need that functionallity?

Thanks,
EricA link to it was added to the first message of this thread.

-Kevin

paulcd2000
06-01-2008, 11:37
I really love the format! much cleaner and easier to use... Please tell me this will become the new standard!

On my only annoyed note: i wish i could see the Main files and Process_data_from_master_uP function. i get antsy if i can't see when everything's being called.

Also, i'd like to re-ask why i would use the Spin functions (maybe if i could see main.c, and i could see when they're bein called...)

Kevin Watson
06-01-2008, 13:28
On my only annoyed note: i wish i could see the Main files and Process_data_from_master_uP function. i get antsy if i can't see when everything's being called.

Also, i'd like to re-ask why i would use the Spin functions (maybe if i could see main.c, and i could see when they're bein called...)The code you're looking for is located in ifi_frc.c.

-Kevin

CPress
06-01-2008, 15:42
Should I be using MPLAB 8 or the 7.2 that came with the kit of parts?

EHaskins
06-01-2008, 16:46
Should I be using MPLAB 8 or the 7.2 that came with the kit of parts?

You should use MPLab 8 if you don't wan to have compatibility issues, but you can use 7.2.

CPress
06-01-2008, 17:34
Where can I get that?

billbo911
06-01-2008, 18:03
Where can I get that?

Please define "that".

If you mean MPLAB 8.0 and C18 v3.1, please read the very first page.
If you are referring to MPLAB 7.4 and C18 v2.7, look in the KOP for the CBOT CD.

JohnC
06-01-2008, 23:20
For those who have not already installed the MCC upgrade, make sure you put it in the same directory with the upgraded MPLAB 8.0. Otherwise the linker can't find the libraries. I know by the trying it the wrong way.

For people having issues with new MPLAB IDE and new C18 playing nice together:

When installing/updating your C18 compiler, (I went with 3.15 and am going to add the #endif that was mentioned earlier (http://www.chiefdelphi.com/forums/showpost.php?p=664164&postcount=27)) there is a page of options after choosing which components to install that has the following:

[ ] Update MPLAB IDE to use this MPLAB C18
[ ] Update MPLAB IDE to use this MPLINK Linker, MPLIB Librarian, and MPASM Assembler

Check them! ;)

SuspectZero
07-01-2008, 00:45
For people having issues with new MPLAB IDE and new C18 playing nice together:

When installing/updating your C18 compiler, (I went with 3.15 and am going to add the #endif that was mentioned earlier (http://www.chiefdelphi.com/forums/showpost.php?p=664164&postcount=27)) there is a page of options after choosing which components to install that has the following:

[ ] Update MPLAB IDE to use this MPLAB C18
[ ] Update MPLAB IDE to use this MPLINK Linker, MPLIB Librarian, and MPASM Assembler

Check them! ;)

Edit: apparently that doesn't actually do the trick. I still get this:
MPLINK 4.15, Linker
Copyright (c) 2007 Microchip Technology Inc.
Error - could not find file 'clib.lib'.
Errors : 1

Link step failed.

for the clib.lib error go to Project> Build Options> Project> Directories Tab. in the Show Directories menu, select library search path. delete the old one and create a new one where the clib.lib is located (a simple search on your computer should be able to find it). i dont know if you should but you can also do the same with the Linker-Script Search path, in which you should delete the old directory location and create a new one going to the lkr folder in your mcc18 folder. that should fix the problem :)

can someone point me in the direction of a tutorial in which i can use this code to program our new 2008 bot? also is there a way of reading the printf statements in an executed program without having to upload onto our robot?

Guy Davidson
07-01-2008, 01:01
can someone point me in the direction of a tutorial in which i can use this code to program our new 2008 bot? also is there a way of reading the printf statements in an executed program without having to upload onto our robot?

If you take Kevin's simple code and just read it, there's a fair chance you'll be able to do the simple stuff (i.e. left joystick forward = left motor forward) by yourself pretty easily. Reading the aliases in ifi_frc and seeing that the joysticks are referred do as p1_y for the y-axis, for example, and that pwm channels are simply pwm01 through 16 also helps. Where you want to start writing code is the Teleop() function in teleop.c, as that is the function that is called upon receiving data in the teleoperated mode.

As far as prtinf goes, if you place your robot on blocks (so that the wheels are spinning freely and not touching anything) and you keep the programming cable connected to the computer, you will see the results of all printf's in the terminal window that pops up after you're done downloading code usinng IFI Loader.

Hope this helps.

jtdowney
07-01-2008, 11:42
The officially supplied version is 2.4, but a free upgrade to 3.10 (don't use 3.15) is available on Microchip's website (http://ww1.microchip.com/downloads/en/DeviceDoc/MPLAB-C18-Upgrade-doc-v3_10.exe). I haven't tested it yet, but the freebie student version will probably work just fine. For those who want to stick with 2.4, I just built a version that will work with the older compiler. After I do a bit of testing, I'll post a link to it.

-Kevin

I can confirm that as long as you make the change Mark McLeod described in this post (http://www.chiefdelphi.com/forums/showpost.php?p=664164). The C18 3.15 Student Edition compiler works just fine.

paulcd2000
07-01-2008, 16:03
Hey, i really like the look! I'm having this problem when i try to compile:

MPLINK 4.1, Linker
Copyright (c) 2006 Microchip Technology Inc.
Error - could not find file 'C:\2008 FRC\frc_library_24\ifi_frc_8722_24.lib'.
Errors : 1


how to fix?

EDIT: Fixed it (for anyone else, add the library file again (project menu) wherever you installed it)

now i'm getting:

MPLINK 4.1, Linker
Copyright (c) 2006 Microchip Technology Inc.
Error - Coff file format for 'ifi_frc_library.o' is out of date.
Error - Could not build member 'ifi_frc_library.o' in library file 'Z:\My Documents\robotics\ifi_frc_24\ifi_frc_8722_24.lib' .
Errors : 2

EHaskins
07-01-2008, 16:13
Hey, i really like the look! I'm having this problem when i try to compile:

MPLINK 4.1, Linker
Copyright (c) 2006 Microchip Technology Inc.
Error - could not find file 'C:\2008 FRC\frc_library_24\ifi_frc_8722_24.lib'.
Errors : 1


how to fix?

The project file is messed up a little. I attached a zipped copy of the .mcp file for the gyro and non-gyro versions. Replace the existing one with that one, and it should work.

THIS IS .MCP FILES ONLY! NO CODE!

EDIT: Are you using c18 v3+ to compile the v2.4 code?

paulcd2000
07-01-2008, 16:18
EDIT: Are you using c18 v3+ to compile the v2.4 code?

i believe i am. you download that from the microchip website? i have...
V 3.1

EHaskins
07-01-2008, 16:19
i believe i am. you download that from the microchip website? i have...
V 3.1

If you are using v3.1 you need to download the verison of the code for c18 v3+. It is listed on the first post of this thread.

paulcd2000
07-01-2008, 16:23
i did download that one.

EDIT: oops... i didn't! Ok, well which of the various links is it? is it "the snapshot of the latest build"

EHaskins
07-01-2008, 16:27
i did download that one.

The library file reference in the error you posted is the v2.4 version.(you can tell by the "_24" ending of the library file and the project directory) Unless kevin included the wrong library you accidentally grabed the wrong file.

The first three links are 3+. The other two are v2.4.

Guy Davidson
07-01-2008, 16:44
Kevin,

In the gyro.h file there are definitions for several different gyros. However, the new KoP gyro is not one of them. Do you think you will be able to add a set of constants for it next time you rebuild?

Thanks.

B.Johnston
07-01-2008, 20:10
Sorry,

Not to drag everyone back to v2.4... but

Ive tried both a last years install and this years mplab v7.2 with 2.4 and it won't even open the 2.4 verion of the beta's project file.

There has been previous mention of mplab7.4 with 2.7.

This is not the version in our Canadian Kits.

Could we please clarify?

Kevin Watson
07-01-2008, 20:44
Kevin,

In the gyro.h file there are definitions for several different gyros. However, the new KoP gyro is not one of them. Do you think you will be able to add a set of constants for it next time you rebuild?

Thanks.I believe the KOP gyro has the same sensitivity (the important parameter to match) as the ADXRS150, which is already supported. I'll look into it and get back to you...

-Kevin

Kevin Watson
07-01-2008, 20:51
Sorry,

Not to drag everyone back to v2.4... but

Ive tried both a last years install and this years mplab v7.2 with 2.4 and it won't even open the 2.4 verion of the beta's project file.

There has been previous mention of mplab7.4 with 2.7.

This is not the version in our Canadian Kits.

Could we please clarify?All versions of the code have project files created with version 8.00 of MPLAB. You can either upgrade to 8.0 for free or just use the project wizard to create a new project. Sorry about the hassle, but it seems like every new version of MPLAP uses a different format for the project files.

-Kevin

TimeOut
07-01-2008, 22:52
Just grabbed both 2.4 versions and opened them up with MPLAB 7.62. I only had two issues.

1) The ifi_frc_8722_25.lib file was being displayed as "Not Found" even though it was in the root directory of the project along with all the other files. I just removed the "Not Found" version and added it back in the project.

2) Error during linking saying that the CLIB.LIB wasn't found. It was looking in c:\program files\mplab\mcc18\lib instead of c:\mcc18\lib. Very likely a configuration mistake on my part here. I just went in to the project properties and added the correct library path.

Compiled and linked without any errors.

Nice work Kevin.

Sean

Kevin Watson
07-01-2008, 23:14
Kevin,

In the gyro.h file there are definitions for several different gyros. However, the new KoP gyro is not one of them. Do you think you will be able to add a set of constants for it next time you rebuild?

Thanks.I just verified the 2008 KOP gyro is 12.5 mV per degree/second, which is the same as the ADXRS150. The datasheet is attached.

-Kevin

Guy Davidson
07-01-2008, 23:52
I just verified the 2008 KOP gyro is 12.5 mV per degree/second, which is the same as the ADXRS150. The datasheet is attached.

-Kevin

Thank you very much. I'll hopefully be able to provide gyro and encoder code feedback soon.

DemonYawgmoth
08-01-2008, 17:21
I've finally been able to compile the 3.1 code, so I started looking over it. It seems that it's a lot cleaner than the past few years' code, but I'm used to those, so I've got a few questions. In the past few years we just put our processing of inputs on the joysticks in Default_Routine(). (This is probably not the best way to have done it, but oh well) I'm assuming we should put all of this code such as using the buttons and stuff into Teleop()? Thanks for the help, great code!

-Arty

Lafleur
08-01-2008, 22:35
Kevin:

In your beta code 3 you have two definition for timer 4, one in the timers.c with its ISR in interrupts.c and a 2nd in the adc.c code again with its ISR...

this maybe confusing for user...

thanks...

ps: there are a few typo still in ADC code, ie label is timer 2, but your code is timer 4...

billbo911
08-01-2008, 22:59
I just spoke with the programming team tonight. We had to decide between EasyC Pro, MPLAB and IFI default code and the newer MPLAB and compiler with Kevin's new build.
I am happy to say, 2073 will be stretching their skills and using the new code and compiler.:D

Now I have a dilemma.
The team does not have access to a laptop, plenty of computers, but no laptop. So, they will have nothing to take with them to competition. I will not be able to join them at our regional either because of a previously arranged vacation in Hawaii. :yikes:
I do have a laptop I am willing to let them borrow, but it currently has my personal copy of MPLAB 7.2 and C18 v2.4 on it that I use with my personal Vex kit. I prefer not to uninstall it if I can avoid it. So, the question is, can MPLAB's 7.2 and 8.1, and C18's v2.4 and v3.1 co-reside on the same system?

Kevin Watson
08-01-2008, 23:02
Kevin:

In your beta code 3 you have two definition for timer 4, one in the timers.c with its ISR in interrupts.c and a 2nd in the adc.c code again with its ISR...

this maybe confusing for user......There's a method to my madness (honest). The new scheme standardizes the ISR names across different code modules to make it easier for me to write code that just drops into someone's project without them having to enter the ISR code directly into their code. The downside is that they need to enable two #defines instead of one (see comments at the top of ifi_frc.h, timers.h and interrupts.h). There is also the possibility that two ISRs with the same name will be enabled, but the compiler will complain. I have another solution that uses linked-list ISRs and self modifying code, but I decided it might become a support nightmare, so I'm not going to release it.

ps: there are a few typo still in ADC code, ie label is timer 2, but your code is timer 4...Thanks. I'll go code splunking and fix 'em

-Kevin

Kevin Watson
08-01-2008, 23:08
I'm assuming we should put all of this code such as using the buttons and stuff into Teleop()?Yep. You can also call IFI's old default_routine(), which has been moved to ifi_code.c.

-Kevin

Kevin Watson
08-01-2008, 23:24
I do have a laptop I am willing to let them borrow, but it currently has my personal copy of MPLAB 7.2 and C18 v2.4 on it that I use with my personal Vex kit. I prefer not to uninstall it if I can avoid it. So, the question is, can MPLAB's 7.2 and 8.1, and C18's v2.4 and v3.1 co-reside on the same system?I installed 2.4 and then made a copy of the entire mcc18 directory. I then applied the 3.10 upgrade to the original 2.4 directory. Now I can use both compilers and all I have to do is make sure the correct directory is named "mcc18" at build time (thanks go to Mark at IFI for pointing this out to me).

-Kevin

billbo911
09-01-2008, 01:38
I installed 2.4 and then made a copy of the entire mcc18 directory. I then applied the 3.10 upgrade to the original 2.4 directory. Now I can use both compilers and all I have to do is make sure the correct directory is named "mcc18" at build time (thanks go to Mark at IFI for pointing this out to me).

-Kevin


Excellent! Got it running!!
Next question. You currently have 3 versions for 3.0: Basic, Basic + Gyro, Basic + encoders.
To add the encoders to the basic + gyro, do I just add the encoder.h/c files to the project? Then follow your instructions for adding your encoder code from an earlier build, like 2007's ifi_encoder.zip (http://www.kevin.org/frc/frc_encoder.zip)?

Kevin Watson
09-01-2008, 02:23
To add the encoders to the basic + gyro, do I just add the encoder.h/c files to the project? Then follow your instructions for adding your encoder code from an earlier build, like 2007's ifi_encoder.zip (http://www.kevin.org/frc/frc_encoder.zip)?It should be a matter of:
1) Adding encoder.c/.h to the project.
2) Putting #include "encoder.h" at the top of each source file where any encoder functions will be called.
3) Calling the encoder initialization function(s) from teleop.c/Initialization().
4) Enable the individual encoder channels at the top of encoder.h.
5) Assign the encoder phase b signals to digital inputs at the top of encoder.h.
6) Enable the interrupt(s) associated with each enabled encoder channel at the top of ifi_frc.h.
7) Make sure there are no conflicting ISRs enabled at the top of interrupts.h.
8) Compile and test.

-Kevin

eugenebrooks
09-01-2008, 03:32
Kevin,

Did anything turn up on your inspection of the provided
code with respect to a possible source of the 8.3 volt bug?

With all the interrupts we will have flying this year,
we are sweating it, although we will get a good six
weeks to see if it surfaces again.

Eugene

Kevin Watson
09-01-2008, 10:25
Kevin,

Did anything turn up on your inspection of the provided
code with respect to a possible source of the 8.3 volt bug?

With all the interrupts we will have flying this year,
we are sweating it, although we will get a good six
weeks to see if it surfaces again.

EugeneWhile testing the new library code, I discovered that I could induce the 8.3 volt bug pretty reliably (I had never seen it before), and more importantly, eliminate it by initializing the entire RAM array to zero at boot time (something the C18 startup code does not do and IFI only partially does). I need to get in with a hardware debugger to see what's going on, but I haven't had time to build the custom cable I need to use the internal debugging header. I hope to get to it sometime in the next week.

If it's of any interest, I did notice the new code and compiler seem to handle high interrupt loads better than with the 2.4 compiler. I setup my ADC code to sample at 6400Hz (each sample generates an interrupt) while generating a few hundred more interrupts/sec with a encoder. The RC had no problems sending a bunch of telemetry using printf() under the load.

-Kevin

Guy Davidson
09-01-2008, 10:33
Excellent! Got it running!!
Next question. You currently have 3 versions for 3.0: Basic, Basic + Gyro, Basic + encoders.
To add the encoders to the basic + gyro, do I just add the encoder.h/c files to the project? Then follow your instructions for adding your encoder code from an earlier build, like 2007's ifi_encoder.zip (http://www.kevin.org/frc/frc_encoder.zip)?

Although Kevin already responded, I felt I should say how I did it. I found a freeware diff utility for windows and compared the two folders, and I used that to see the changes and add one to the other. It worked pretty flawlessly and also helped me integrate the two test teleop() routines.

DemonYawgmoth
09-01-2008, 11:22
Yep. You can also call IFI's old default_routine(), which has been moved to ifi_code.c.

-Kevin

Awesome, thanks a lot Kevin!

Mike Mahar
09-01-2008, 12:05
Does this mean that the resolution that IFI published to "fix" the 8.3 volt problem does not work. They recommend changing a high chuck of memory (0xF00 - 0xF5f) to "PROTECTED".

Kevin Watson
09-01-2008, 16:50
Does this mean that the resolution that IFI published to "fix" the 8.3 volt problem does not work. They recommend changing a high chuck of memory (0xF00 - 0xF5f) to "PROTECTED".No, that's not what I'm saying. Clearly the "fix" does work because I don't think anyone has seen the 8.3 volt problem since IFI published the new linker script.

I just thought it was interesting that a bug in my code could induce the same symptom that teams saw two years ago. Although I know how to fix it, I'm not sure I know exactly what caused it. For my own edification I'd like to track it down because it's not obvious to me what is causing it by examining the source code.

-Kevin

billbo911
09-01-2008, 17:17
I just thought it was interesting that a bug in my code could induce the same symptom that teams saw two years ago. Although I know how to fix it, I'm not sure I know exactly what caused it. For my own edification I'd like to track it down because it's not obvious to me what is causing it by examining at the source code.

-Kevin

I have complete faith in your ability to find the root cause! But, until you do, and because you know how to fix it, can you share with us how to fix it so we don't run into it until the final fix is released. From your prior comments, it sounds like we just need to completely clear the RAM on reboot. Could we make the changes to our projects to do this, or would it just be easier to replace a particular file with the mods in it already?

BTW, I fully understand your need to know exactly why this problem happens and just knowing how to fix it isn't enough. ;)

Kevin Watson
09-01-2008, 17:28
I have complete faith in your ability to find the root cause! But, until you do, and because you know how to fix it, can you share with us how to fix it so we don't run into it until the final fix is released. From your prior comments, it sounds like we just need to completely clear the RAM on reboot. Could we make the changes to our projects to do this, or would it just be easier to replace a particular file with the mods in it already?

BTW, I fully understand your need to know exactly why this problem happens and just knowing how to fix it isn't enough. ;)As far as I can tell, there are no bugs in my code. The bug I referred to was fixed long ago and the fix incorporated before I released any code. If interested, it's the six lines of assembly under the "initialize memory to all zeros" comment in ifi_frc.c/_startup().

-Kevin

billbo911
09-01-2008, 17:36
As far as I can tell, there are no bugs in my code. The bug I referred to was fixed long ago and the fix incorporated before I released any code. If interested, it's the six lines of assembly under the "initialize memory to all zeros" comment in ifi_frc.c/_startup().

-Kevin

Ahh, got it. Your code has this problem corrected already. The error was in older code. COOL!!! I feel better already!

ayeckley
09-01-2008, 22:28
There seems to be something amiss when using the combination of MPLAB 8.0, C18 V2.4, the V2.4 beta code and the MPLAB SIM debugger. I think that's a "legal" combination, right? The simulator seems to be going off into the weeds when it hits line 373 (serial port enable) of serial_ports.c. The odd thing is that if you step through it it hangs up on that line, but if you let it run it instead hangs up on line 427 of ifi_frc.c, which is _do_cinit();

Is this just a question of the beta code not supporting MPLAB SIM? I don't think I've seen any _SIMULATOR definitions in either the 2.4 or the 3.1 beta code.

Edit: The same thing happens when I run the 3.1 beta code with C18 v3.1. Maybe I'm the only one trying to use MPLAB SIM? It worked like a champ with last year's code.

eugenebrooks
10-01-2008, 01:53
Does this mean that the resolution that IFI published to "fix" the 8.3 volt problem does not work. They recommend changing a high chuck of memory (0xF00 - 0xF5f) to "PROTECTED".

The IFI "fix" did not help the instance of the 8.3 volt problem
that we ran into using the 2006 controller in the game Aim High.
If may have helped some teams, but it did not fix things
for us and I am aware of some other teams that it did not fix
the problem for as well.

We will happily try zeroizing all of memory at startup if we
see 8.3 showing up in the voltage display during this build season.

Eugene

Kevin Watson
10-01-2008, 02:40
The IFI "fix" did not help the instance of the 8.3 volt problem that we ran into using the 2006 controller in the game Aim High. If may have helped some teams, but it did not fix things for us and I am aware of some other teams that it did not fix the problem for as well.Ugh, I spoke with IFI last week and they're of the opinion that the 8.3 volt bug has been fixed. If this bug bites you again, please let IFI know (and if you don't mind, me too).

We will happily try zeroizing all of memory at startup if we see 8.3 showing up in the voltage display during this build season. Open up ifi_startup.c and replace the call to Clear_Memory() with this code:

// initialize memory to all zeros
_asm
lfsr 0, 0
movlw 0xF
clear_loop:
clrf POSTINC0, 0
cpfseq FSR0H, 0
bra clear_loop
_endasm


-Kevin

Joe Ross
10-01-2008, 09:56
Ugh, I spoke with IFI last week and they're of the opinion that the 8.3 volt bug has been fixed. If this bug bites you again, please let IFI know (and if you don't mind, me too).


I did talk to IFI last year, after seeing the bug. However, they were busy fixing radios at that time. There was at least one other team with the problem as well.

You can read about what I found in this thread, starting at post 133. http://www.chiefdelphi.com/forums/showthread.php?t=44954&page=9

neutrino15
10-01-2008, 21:12
I am currently in the process of testing the old configure.py makefile with this new code. Has anybody had success?

I use a mac, and seriously do not want to try and update mplab in vmware.. painful memories..

Nathans
10-01-2008, 23:13
Our mistake here is probably very silly and obvious, but the solution eludes us nevertheless.

We're trying to get the gryo and accelerometer working, but we get a build error when we try to use the Get_Analog_Value function.

gyro = Get_Analog_Value(rc_ana_in16);

The build error we get is:

Error [1105] symbol 'ADC_CH15' has not been defined

I've gotten as far as the fact that rc_ana_in16 is aliased to ADC_CH15, but no further.

comphappy
11-01-2008, 01:12
I am trying to figure out the encoder code, and do not understand how the code knows what way the shaft is turning,
void Int_1_ISR(void)
{
if(ENCODER_1_PHASE_B_PIN == 0)
{
Encoder_1_Count -= ENCODER_1_TICK_DELTA;
}
else
{
Encoder_1_Count += ENCODER_1_TICK_DELTA;
}
}

What really is ENCODER_1_PHASE_B_PIN?
And what port is this interrupt running one Input 1 or 2?

billbo911
11-01-2008, 01:34
I am trying to figure out the encoder code, and do not understand how the code knows what way the shaft is turning,
void Int_1_ISR(void)
{
if(ENCODER_1_PHASE_B_PIN == 0)
{
Encoder_1_Count -= ENCODER_1_TICK_DELTA;
}
else
{
Encoder_1_Count += ENCODER_1_TICK_DELTA;
}
}

What really is ENCODER_1_PHASE_B_PIN?
And what port is this interrupt running one Input 1 or 2?


When using a "Quadrature" encoder, there are two signals generated, Phase A and Phase B. Both are square waves with "B" 180 degrees out of phase from "A". When triggered by "A" going high, the ISR looks at "B", if it is low, you are going forward, if "B" is high you are going backward.
In the code it explains how to attach the A and B phases.

Now, if you are not using a quadrature encoder but one with a single output, like the GTS in the kit, then determining direction is a much bigger challenge although, not impossible.

Kevin Watson
11-01-2008, 01:35
I am trying to figure out the encoder code, and do not understand how the code knows what way the shaft is turning,
void Int_1_ISR(void)
{
if(ENCODER_1_PHASE_B_PIN == 0)
{
Encoder_1_Count -= ENCODER_1_TICK_DELTA;
}
else
{
Encoder_1_Count += ENCODER_1_TICK_DELTA;
}
}

What really is ENCODER_1_PHASE_B_PIN?
And what port is this interrupt running one Input 1 or 2?Open up encoder.h and have a look at the comments for the answer to what ENCODER_1_PHASE_B_PIN is. Rotation direction is determined by the logic state of phase b on the rising edge of phase a. Have a look at this illustration (http://www.kevin.org/frc/encoder/encoder_isr_latency.pdf) for a diagram showing what the waveform looks like. If the encoder were spinning in the opposite direction, the phase b waveform in the illustration would be flipped top to bottom.

-Kevin

comphappy
11-01-2008, 01:49
Open up encoder.h and have a look at the comments for the answer to what ENCODER_1_PHASE_B_PIN is. Rotation direction is determined by the logic state of phase b on the rising edge of phase a. Have a look at this illustration (http://www.kevin.org/frc/encoder/encoder_isr_latency.pdf) for a diagram showing what the waveform looks like. If the encoder were spinning in the opposit direction, the phase b waveform in the illustration would be flipped top to bottom.

-Kevin
I got what port it was hooked up to, I was just trying to figure out what phase b was, as the GTS does not have one, I will just modify the code so that instead of looking at phase b it will look at the PWM > or < 127 to determine the correct operand. I just need to determine the distance travailed on the linear drive.
Thank you for the diagram it was really useful. (BTW I like your code, it is clean and well organized)

Mike Mahar
11-01-2008, 08:39
Our mistake here is probably very silly and obvious, but the solution eludes us nevertheless.

We're trying to get the gryo and accelerometer working, but we get a build error when we try to use the Get_Analog_Value function.

gyro = Get_Analog_Value(rc_ana_in16);

The build error we get is:

Error [1105] symbol 'ADC_CH15' has not been defined

I've gotten as far as the fact that rc_ana_in16 is aliased to ADC_CH15, but no further.

The simple answer is don't use Get_Analog_value. The new code in adc.c should replace it. Instead you should call Get_ADC_Result. Make sure you set NUM_ADC_CHANNELS to the number of analog inputs you are using and allocate them starting with first one and go up from there.

The analog inputs are not instantaneous. You have to start them, then wait a certain amount of time and then read them. The code in adc.c will do that for you and uses a timer to wait. Calls to Get_ADC_Result will simply fetch the result that the library has already captured for you.

The function Get_Analog_value does the start/wait/read operation in-place and takes longer and consumes valuable cycles that could be put to better use doing something else.

Mike Mahar
11-01-2008, 09:06
Kevin, I have question about adc.c.
In Timer_4_ISR you have two for loops that cover the same range of indexes.
if(samples >= adc_samples_per_update)
{
// update the ADC result array
for(i=0; i < num_adc_channels; i++)
{
adc_result[i] = (long)(accum[i] >> adc_result_divisor);
}
// reset the sample accumulator(s) to zero
for(i=0; i < num_adc_channels; i++)
{
accum[i] = 0L;
}
// signal that a fresh sample set is available
adc_update_count++;

// start a fresh sample set
samples = 0;
}


Is there some reason that you don't reset the sample accumulator to 0 in the first loop?
if(samples >= adc_samples_per_update)
{
// update the ADC result array
for(i=0; i < num_adc_channels; i++)
{
adc_result[i] = (long)(accum[i] >> adc_result_divisor);
// reset the sample accumulator to zero
accum[i] = 0L;
}

// signal that a fresh sample set is available
adc_update_count++;

// start a fresh sample set
samples = 0;
}


I'd think that any cycles saved in an ISR is a good thing.

billbo911
11-01-2008, 10:03
.... as the GTS does not have one, I will just modify the code so that instead of looking at phase b it will look at the PWM > or < 127 to determine the correct operand. I just need to determine the distance travailed on the linear drive....

I have used this method in the past on my Vex bot. It works fairly well but is not perfect. For use in straight line driving, as in Hybrid mode, it should be sufficient. BTW, make sure you account for the dead-band associated with the Victor.

Joe Ross
11-01-2008, 10:09
I am currently in the process of testing the old configure.py makefile with this new code. Has anybody had success?

I use a mac, and seriously do not want to try and update mplab in vmware.. painful memories..

There have been at least two makefiles posted in this thread.

Kevin Watson
11-01-2008, 16:37
Is there some reason that you don't reset the sample accumulator to 0 in the first loop?Beats me. I'll have a look at it when I get home from work.

-Kevin

Lafleur
11-01-2008, 18:07
Kevin:

An idea...

A quick and dirty utilizations counter...

Add a global long integer to you project called "cycle". At the end of the while(TRUE) in main.c, do a cycle++ to build a loop counter, this will tell you how many time you have been in the while(TRUE) loop.

In the 26.4ms loop, read the cycle counter, clear it and divide by (264k clock cycles, the number of clock cycles in 26.4ms - the number of cycles used in the while(TRUE) ) This will give you a crude % utilization of the CPU that the teams can use to check on there code efficiency...


also, the counter could be scaled so that one could use an unsigned int

Loki1989
11-01-2008, 19:37
how exactically are we supposed to build in 7.21 with the c18 3.10 and now i can't just push F10 to build.

edit:

also when I open it it does not open correctly when I try to open the project files.

Nathans
11-01-2008, 19:47
The simple answer is don't use Get_Analog_value. The new code in adc.c should replace it. Instead you should call Get_ADC_Result. Make sure you set NUM_ADC_CHANNELS to the number of analog inputs you are using and allocate them starting with first one and go up from there.

The analog inputs are not instantaneous. You have to start them, then wait a certain amount of time and then read them. The code in adc.c will do that for you and uses a timer to wait. Calls to Get_ADC_Result will simply fetch the result that the library has already captured for you.

The function Get_Analog_value does the start/wait/read operation in-place and takes longer and consumes valuable cycles that could be put to better use doing something else.

How do you allocate the analog inputs with Get_ADC_Result? I found where to set NUM_ADC_CHANNELS, but I haven't found anything about allocating specific inputs. Furthermore, what is used as the argument of Get_ADC_Result?

Lafleur
12-01-2008, 00:07
Kevin:

An idea...

A quick and dirty utilizations counter...

Add a global long integer to you project called "cycle". At the end of the while(TRUE) in main.c, do a cycle++ to build a loop counter, this will tell you how many time you have been in the while(TRUE) loop.

In the 26.4ms loop, read the cycle counter, clear it and divide by (264k clock cycles, the number of clock cycles in 26.4ms - the number of cycles used in the while(TRUE) ) This will give you a crude % utilization of the CPU that the teams can use to check on there code efficiency...



also, the counter could be scaled so that one could use an unsigned int


an error in my logic....

you also need to take the number of loops and multiply by the number of cycles in the loop and use this to subtract from the total number of cycles in the 26ms...


tom lafleur

This is the code I did in a TEST module with only the ATD and Serial ports, I have a timer that interrupts me ever 100ms. I disable all interrupts and use the MPLAB simulator to measure the number of cycles in the main loop. In my case it was 42 cycles. Note, division is only by 10,000 and not 1,000,000 to give a % with out the need to multiply by 100, saving some cycles.

int util;
long int cycle = 0; is a global in the code, it needs to be long because of the multiplication need a long

in the main loop I have a cycle++;

// here we will calculate the % of the CPU we are using
// we are here every 100ms, that 1,000,000 cycles of the processor at 40MHz
// utilization = the number of times in the do nothing loop, * the number of cycles in
// the main loop. 42 is current number of cycles in main loop

util = 100 - ((cycle * 42)/10000);
cycle = 0;

Kevin Watson
12-01-2008, 00:19
Kevin:

An idea...

A quick and dirty utilizations counter...

Add a global long integer to you project called "cycle". At the end of the while(TRUE) in main.c, do a cycle++ to build a loop counter, this will tell you how many time you have been in the while(TRUE) loop.

In the 26.4ms loop, read the cycle counter, clear it and divide by (264k clock cycles, the number of clock cycles in 26.4ms - the number of cycles used in the while(TRUE) ) This will give you a crude % utilization of the CPU that the teams can use to check on there code efficiency...


also, the counter could be scaled so that one could use an unsigned intCool. I've been thinking about adding a few cool features like a system clock (uS?, mS?, real time?) and scheduler. Perhaps I should start another thread for wish list items?

-Kevin

wireties
12-01-2008, 10:39
A Makefile (forced to call it Makefile.txt to upload) for Linux in included. I'm using cxoffice (a canned wine setup) but it can be easily altered to work with plain old wine or to run under Windoze. This compiled Kevin's latest stuff with no errors but with a bunch of signed/unsigned warnings. I have not run the code on the robot yet!


Enjoy

bronxbomber92
12-01-2008, 10:50
Hi,

EDIT - We've got a working install of mplab 8.00, but we can't seem to get 3.00 installed. We run through the installer fine, but it doesn't seem to actually install.. MPLAB is still using 2.40, and we can't find the installation directory of 3.00 - only 2.40.

I'm trying to install mplab 8.00 and the 3.0 compiler, I downloaded the software on the first post, and the tried running it over the install of mplab 7.20. It seemed to install, but now when ever we open mplab it says 8.00 is not enabled, but we can still use 7.20. How is the best way to install it and the compiler with pre-existing versions all ready installed?

NetElemental
12-01-2008, 12:14
Kevin, First of all thanks, this will be a great help.

Our team has a 2005 robot controller and operator interface mounted to a board for testing purposes when it is not practical to use the current robot controller (e.g., the current controller is on the robot, and the robot is being built or modified). We swapped out the libraries and linker files for the ones corresponding to the PIC18F8520, changed the target to the 8520, and tried to compile. We got this error(in your ifi_frc_gyro code, if it's relevant):

C:\Users\NetElemental\Documents\Code\ifi_frc_gyro\ ifi_code.c:33:Error [1105] symbol 'ifi_analog_channels' has not been defined
inaccuracy

The line code corresponds to

29 #if defined(__18F8722)
30 OpenADC( ADC_FOSC_RC & ADC_RIGHT_JUST & ADC_0_TAD,
31 ADC_channel & ADC_INT_OFF & ADC_VREFPLUS_VDD & ADC_VREFMINUS_VSS,15);
32 #else
33 OpenADC( ADC_FOSC_RC & ADC_RIGHT_JUST & ifi_analog_channels,
34 ADC_channel & ADC_INT_OFF & ADC_VREFPLUS_VDD & ADC_VREFMINUS_VSS );
35 #endif


Any ideas? We couldn't find ifi_analog_channels in either .lib file. What would your suggestion be?

Thanks,
James

Lafleur
12-01-2008, 12:31
CPU Utilization

In a quick test I did today, using a ATD sample rate of 200Hz, I had 19% utilizations of the processor. Changing only the ATD sample rate to 6400Hz, utilization went to 43%.

Kevin Watson
12-01-2008, 13:22
Kevin, First of all thanks, this will be a great help.

Our team has a 2005 robot controller and operator interface mounted to a board for testing purposes when it is not practical to use the current robot controller (e.g., the current controller is on the robot, and the robot is being built or modified). We swapped out the libraries and linker files for the ones corresponding to the PIC18F8520, changed the target to the 8520, and tried to compile. We got this error(in your ifi_frc_gyro code, if it's relevant):

C:\Users\NetElemental\Documents\Code\ifi_frc_gyro\ ifi_code.c:33:Error [1105] symbol 'ifi_analog_channels' has not been defined
inaccuracy

The line code corresponds to

29 #if defined(__18F8722)
30 OpenADC( ADC_FOSC_RC & ADC_RIGHT_JUST & ADC_0_TAD,
31 ADC_channel & ADC_INT_OFF & ADC_VREFPLUS_VDD & ADC_VREFMINUS_VSS,15);
32 #else
33 OpenADC( ADC_FOSC_RC & ADC_RIGHT_JUST & ifi_analog_channels,
34 ADC_channel & ADC_INT_OFF & ADC_VREFPLUS_VDD & ADC_VREFMINUS_VSS );
35 #endif


Any ideas? We couldn't find ifi_analog_channels in either .lib file. What would your suggestion be?

Thanks,
JamesReplace "ifi_analog_channels" with "ADC_15ANA" and it should compile.

-Kevin

Kevin Watson
12-01-2008, 14:43
Replace "ifi_analog_channels" with "ADC_15ANA" and it should compile.I just tried it here and you probably need to add this line at the top of the file: "#include <adc.h>". You can also just comment out the line and compile. On my to do list is replacement code for this function.

-Kevin

NetElemental
12-01-2008, 14:49
Kevin,
Thanks - we already had #include <adc.h>, it seemed to come like that. Our mcc18 adc.h file actually did not have a definition for ADC_15ANA - it had 0-14 and 16. Assuming the intent is to set all the pins to analog, I changed "ifi_analog_channels" to "ADC_16ANA" if it helps.

In our file, we had

#define ADC_14ANA 0b11110001 // analog: AN0->13 digital: AN14->15
#define ADC_16ANA 0b11110000 // All analog

Weird, eh?

Thanks again,
James

Kevin Watson
12-01-2008, 15:09
Kevin,
Thanks - we already had #include <adc.h>, it seemed to come like that. Our mcc18 adc.h file actually did not have a definition for ADC_15ANA - it had 0-14 and 16. Assuming the intent is to set all the pins to analog, I changed "ifi_analog_channels" to "ADC_16ANA" if it helps.

In our file, we had

#define ADC_14ANA 0b11110001 // analog: AN0->13 digital: AN14->15
#define ADC_16ANA 0b11110000 // All analog

Weird, eh?

Thanks again,
JamesLooks like their library documentation is messed-up. Use ADC_16ANA.

-Kevin

Rindill
12-01-2008, 15:13
Using new code with Eclipse? How to make the correct makefile? Help?

Ethan Reesor
12-01-2008, 15:28
Kevin,
I am currently working with the ADC and Gyro code for the mcc18 v3, using MPLAB IDE v8.0. I realized that I had v2.4 of the compiler, so I downloaded and ran the v3.1 updater. Now, when I compile, the linker tells me I have the wrong (out of date) COFF file format for the object files (*.o). Any ideas?

Here is the error:

Error - Coff file format for 'C:\Documents and Settings\Ethan\My Documents\Programming\FIRST\Code\ifi_frc_gyro\adc. o' is out of date.


-Ethan

Kevin Watson
12-01-2008, 15:57
Kevin,
I am currently working with the ADC and Gyro code for the mcc18 v3, using MPLAB IDE v8.0. I realized that I had v2.4 of the compiler, so I downloaded and ran the v3.1 updater. Now, when I compile, the linker tells me I have the wrong (out of date) COFF file format for the object files (*.o). Any ideas?

Here is the error:

Error - Coff file format for 'C:\Documents and Settings\Ethan\My Documents\Programming\FIRST\Code\ifi_frc_gyro\adc. o' is out of date.


-EthanTry using the "build all" option, which will recompile all source files before linking.

-Kevin

Guy Davidson
12-01-2008, 16:07
I am planning on using the custom pwm.c/h code to generate the two pwm signals at 100Hz. I realized that unless I am also providing command and mointoring the system at 100Hz, it won't gain me anything (i.e. if I were to generate pwm commands in the 38Hz loop and update the commands at 100Hz, I don't think I'd gain anything). Hence I am also planning to update encoder-based pid loops at a rate of 100Hz. Not only this will give me a constant interval between updates, making my velocity estimates more reliable, it should make use of the faster update rate by generating commands at that rate.

Will doing something like that impact the performance of the the pwm functions? My PID loop code is not much more than a bunch of logical tests and integer multiplications. I am wondering if I should be okay if I call the PID code from within the Timer ISR that generates the pwm signal, or am I risking a significant drop in performance.

Thanks (and I hope my question is worded clearly enough - if not, please let me know, and I'll try to reword it)

bronxbomber92
12-01-2008, 16:08
This is related to the Beta code, but with default code compiled with MPLAB 8.00 and c18 2.40 I get a similar error to Etahn Reesor:
error - Coff file format for 'ifi_library.o' is out of date
error - Coud not find member 'ifi_library.o' in library file 'C:\filepathToProject\FRC_library_8722.lib'p

Any idea on how to fix this?

Jon236
12-01-2008, 18:31
It should be a matter of:
1) Adding encoder.c/.h to the project.
2) Putting #include "encoder.h" at the top of each source file where any encoder functions will be called.
3) Calling the encoder initialization function(s) from teleop.c/Initialization().
4) Enable the individual encoder channels at the top of encoder.h.
5) Assign the encoder phase b signals to digital inputs at the top of encoder.h.
6) Enable the interrupt(s) associated with each enabled encoder channel at the top of ifi_frc.h.
7) Make sure there are no conflicting ISRs enabled at the top of interrupts.h.
8) Compile and test.

-Kevin

Kevin,

I keep getting a link error
Error - could not find definition of symbol 'Int_6_ISR' in file 'C:\dev\ifi_frc_gyro\ifi_frc.o'.
even when I enabled interrupts 3-6 in ifi_frc.h

Thoughts?

Kevin Watson
12-01-2008, 21:09
Kevin,

I keep getting a link error
Error - could not find definition of symbol 'Int_6_ISR' in file 'C:\dev\ifi_frc_gyro\ifi_frc.o'.
even when I enabled interrupts 3-6 in ifi_frc.h

Thoughts?The linker can't find the Int_6_ISR() function in your source files. You need to perform step #4 in the list and enable encoder #6. If this doesn't solve your problem, can you send me your code (I hope I haven't done something knuckle headed)?

-Kevin

Kevin Watson
12-01-2008, 21:21
I am wondering if I should be okay if I call the PID code from within the Timer ISR that generates the pwm signal, or am I risking a significant drop in performance. Sounds like a fun project. I would not do my PID calculations in a ISR. Instead, I would set up a timer to fire off at 100Hz and then wait for the timer interrupt flag to go high and then do your PID calculations. The ideal place for this code is Teleop_Spin() and/or Autonomous_Spin(). Let me know if you have any problems.

-Kevin

Jon236
12-01-2008, 21:48
The linker can't find the Int_6_ISR() function in your source files. You need to perform step #4 in the list and enable encoder #6. If this doesn't solve your problem, can you send me your code (I hope I haven't done something knuckle headed)?

-Kevin

All my problems went away when I downloaded you encoder_beta version.

comphappy
13-01-2008, 00:13
I have used this method in the past on my Vex bot. It works fairly well but is not perfect. For use in straight line driving, as in Hybrid mode, it should be sufficient. BTW, make sure you account for the dead-band associated with the Victor.

Yes, I am aware of that, but in this case the PWM is being driven with out any user input, should be ether 0 or 255, nothing in the middle.

billbo911
13-01-2008, 13:18
Yes, I am aware of that, but in this case the PWM is being driven with out any user input, should be ether 0 or 255, nothing in the middle.

I'm not sure I am following what the question is. If you have a PWM value that is going to be only one of two states, 0 or 255, then just test for one state. For example:

if (pwm03 > 127)
{
accumulator ++;
}
else
{
accumulator --;
}

I chose 127 just for simplicity.

Just as easily, you could test for the actual value.

if (pwm03 == 255)
{
accumulator ++;
}
else
{
accumulator --;
}

paulcd2000
13-01-2008, 15:12
It has been explained how to add encoders to the gyro code, how about the reverse?

Kevin Watson
13-01-2008, 15:42
It has been explained how to add encoders to the gyro code, how about the reverse?Assuming you want to add the gyro code to your project, lets see if I can do this off the top of my head:

1) Add adc.c/.h and gyro.c/.h to your project directory.
2) Add adc.c/.h and gyro.c/.h to your MPLAB project.
3) Add #include "adc.h" and #include "gyro.h" at the top of teleop.c.
4) Add a call to Initialize_ADC() and Initialize_Gyro() in teleop.c/Initialization().
5) If needed, add #include gyro.h to the top of autonomous.c and/or disabled.c
6) Where needed, add a call to Process_Gyro_Data() in the *_spin() functions.
7) Enable the timer 4 ISR at the top of ifi_frc.h.
8) Make sure timer 4 is disabled at the top of timers.h.
9) Add calibration code somewhere like disabled.c/Disabled() (this requires that you use a mode dongle to emulate the field controller, which will put you in disabled mode for a period of time before transitioning to autonomous mode).
10) Do anything else I forgot to mention here <grin>.
11) Compile, test, calibrate your gyro (see gyro.h).

-Kevin

Rindill
13-01-2008, 16:19
Okay, I got a makefile working.

# Jeff Holland: 2/26/2005
# Manual makefile to go with Eclipse
# NOTE: enviornment variables must be set to find linker & compiler.
# NOTE: runs with GNU Make. Might also run with Microsoft nmake with tweaks.
#
# This Makefile will compile any .c file in the current directory and build the
# (PROJECT).hex file you can load in the robot.
#
# The OBJFILES variable is filled with .o targets using the wildcard
# command and patsubst. The $(wildcard *.c) will retrieve any .c
# file in the current directory and store it in a variable. The patsubstr
# functions is used to convert a file from one format to another. In
# this case each .c file is converted into a .o extension and then
# stored into OBJFILES. This variable is then used to compile each .c file.
# The rule %.o: %.c rebuilds any .o file if the cooresponding .c file has changed.
# In the compile line you will see two variables $@ and $<.
# $@ will match the target and the $< will match the dependency. So, for
# example, $< will contain main.c whenever $@ contains main.o.
# Note: makefiles are counterintuitive in that the rules don't run in the order
# listed in the file, but instead run whenever they are matched. Also be careful
# with tabs and spaces. Rules need a tab (not spaces) before each action.

#RENAME AS NEEDED
PROJECT=RR

#del for windows, rm for unix
RM := del /F

LIB_PATH=".;c:\mcc18\lib"
#MCC18_PATH=c:\mcc18\bin\

LINKER = mplink

CC = mcc18

PICFLAG=-p=18F8722

CFLAGS = -i".;C:\mcc18\h" -nw=2066

DEFINES = -D_FRC_BOARD

#optimization parameters (default = full optimization)
OPT_ENABLE_ALL=
OPT_DEBUG=-Ou- -Ot- -Ob- -Op- -Or- -Od- -Opa-
OPT=$(OPT_ENABLE_ALL)

#make a list of object files that match all C files
OBJFILES := $(patsubst %.c,%.o,$(wildcard *.c))

COMPILE=$(CC) $(PICFLAG) $< -fo=$@ $(CFLAGS) $(DEFINES) $(OPT)

all: $(PROJECT).hex

#re-link if any object file changed
$(PROJECT).hex: $(OBJFILES)
$(LINKER) "18f8722.lkr" $(OBJFILES) "ifi_frc_8722.lib" /l $(LIB_PATH) /m $(PROJECT).map /o $(PROJECT).hex

# Recompile a file if it's c-file changes,
# OR recompile everything if ANY header file changes
%.o: %.c *.h
$(COMPILE)

#delete all the build files so you can start from scratch.
clean:
-$(RM) $(OBJFILES)
-$(RM) $(PROJECT).hex
-$(RM) $(PROJECT).map
-$(RM) $(PROJECT).lst
-$(RM) $(PROJECT).cod

Kevin Watson
13-01-2008, 18:15
Is there some reason that you don't reset the sample accumulator to 0 in the first loop?I just made this change (thanks Mike) and added some additional documentattion to the ADC and gyro code. Link is in message #1 of this thread.

-Kevin

Jon236
13-01-2008, 18:46
Kevin

still getting wierd compile errors.....
Clean: Done.
Executing: "C:\mcc18\bin\mcc18.exe" -p=18F8722 "autonomous.c" -fo="autonomous.o" -k -mL -Ou- -Ot- -Ob- -Op- -Or- -Od- -Opa-
C:\mcc18\h\stdio.h:26:Error [1178] illegal declaration of object of type void
C:\mcc18\h\stdio.h:27:Error [1178] illegal declaration of object of type void
C:\mcc18\h\stdio.h:31:Error [1178] illegal declaration of object of type void
Halting build on first failure as requested.
BUILD FAILED: Sun Jan 13 18:44:53 2008

????????

Kevin Watson
13-01-2008, 19:14
Kevin

still getting wierd compile errors.....
Clean: Done.
Executing: "C:\mcc18\bin\mcc18.exe" -p=18F8722 "autonomous.c" -fo="autonomous.o" -k -mL -Ou- -Ot- -Ob- -Op- -Or- -Od- -Opa-
C:\mcc18\h\stdio.h:26:Error [1178] illegal declaration of object of type void
C:\mcc18\h\stdio.h:27:Error [1178] illegal declaration of object of type void
C:\mcc18\h\stdio.h:31:Error [1178] illegal declaration of object of type void
Halting build on first failure as requested.
BUILD FAILED: Sun Jan 13 18:44:53 2008

????????Send me your code and I'll look at it. Which compiler are you using?

-Kevin

Jon236
13-01-2008, 19:23
It's the ifi_frc_encoder_beta oput of the box....using the 3.10 compiler and 8.0 MpLab

Kevin Watson
13-01-2008, 20:08
It's the ifi_frc_encoder_beta oput of the box....using the 3.10 compiler and 8.0 MpLabIt builds just fine for me. Make sure the tool suite is set correctly and then use the project wizard to rebuild your project.

-Kevin

Jon236
13-01-2008, 20:33
Kevin,

I re-installed 3.10 and it worked just fine!

Thanks for all your work!

Jon

Guy Davidson
13-01-2008, 20:46
Sounds like a fun project. I would not do my PID calculations in a ISR. Instead, I would set up a timer to fire off at 100Hz and then wait for the timer interrupt flag to go high and then do your PID calculations. The ideal place for this code is Teleop_Spin() and/or Autonomous_Spin(). Let me know if you have any problems.

-Kevin

Interesting. You're suggesting I run another timer at 100hz, and use one to time the calls on PWM() and the other one for timing the calculations, right?
I think I might have a simpler solution. I have the ISR that calls on PWM() to set another flag high (not the interrupt flag), and check that flag in the _Spin() functions. As a result, assuming the calculation runs fast enough, I'll be one cycle behind, which would probably be the case either way. Does that sounds like it could work?

Thanks.

Lafleur
13-01-2008, 21:13
I just made this change (thanks Mike) and added some additional documentattion to the ADC and gyro code. Link is in message #1 of this thread.

-Kevin

In testing the code on my test system, this change to the ADC.c code made about 1% saving it time...

comphappy
14-01-2008, 01:29
I just made this change (thanks Mike) and added some additional documentattion to the ADC and gyro code. Link is in message #1 of this thread.

-Kevin


I really appreciate what you are doing, and i don't mean to ask for more, but would it be possible for you to create diff/patch files when you release a new version, to make updating our code easier?

comphappy
14-01-2008, 01:37
I'm not sure I am following what the question is. If you have a PWM value that is going to be only one of two states, 0 or 255, then just test for one state. For example:

if (pwm03 > 127)
{
accumulator ++;
}
else
{
accumulator --;
}

I chose 127 just for simplicity.

Just as easily, you could test for the actual value.

if (pwm03 == 255)
{
accumulator ++;
}
else
{
accumulator --;
}

I was attempting to understand what the PHASE B was, i now know, and was just expressing how i was going to do it with out it in my specialized case. That code you wrote is exactly what i had described, and what has been in our code for a while now.

billbo911
14-01-2008, 09:57
I was attempting to understand what the PHASE B was, i now know, and was just expressing how i was going to do it with out it in my specialized case. That code you wrote is exactly what i had described, and what has been in our code for a while now.

It's nice to know that great minds think alike :yikes:

I can, with 90% certainty, say that this year we will be using both of the GTS's and one quadrature encoder on our bot. Two for the drive, one for the "ball handler". Now, if the budget allows, we may go with three quadratures. (I hope, I hope, I hope :) )

Kevin Watson
14-01-2008, 12:36
Interesting. You're suggesting I run another timer at 100hz, and use one to time the calls on PWM() and the other one for timing the calculations, right?
I think I might have a simpler solution. I have the ISR that calls on PWM() to set another flag high (not the interrupt flag), and check that flag in the _Spin() functions. As a result, assuming the calculation runs fast enough, I'll be one cycle behind, which would probably be the case either way. Does that sounds like it could work?

Thanks.My thought was to not even have an ISR and just keep an eye on your 100Hz timer interrupt flag with code located in teleop_spin() and/or autonomous_spin(). Just setup and start a timer, but don't set the interrupt enable bit to one, which will prevent the processor from calling the low priority ISR. Then with code like this in *_spin():

if(INTCONbits.TMR0IF)
{
// Get encoder counts

// Calculate position/velocity

// Do PID calculations

// Update PWM values

//reset interrupt flag
INTCONbits.TMR0IF = 0;
}

...you can implement your 100Hz contol algorithm.

-Kevin

paulcd2000
14-01-2008, 15:46
thank you kevin! i compiled, and everything checks out!

Guy Davidson
14-01-2008, 15:59
My thought was to not even have an ISR and just keep an eye on your 100Hz timer interrupt flag with code located in teleop_spin() and/or autonomous_spin(). Just setup and start a timer, but don't set the interrupt enable bit to one, which will prevent the processor from calling the low priority ISR. Then with code like this in *_spin():

if(INTCONbits.TMR0IF)
{
// Get encoder counts

// Calculate position/velocity

// Do PID calculations

// Update PWM values

//reset interrupt flag
INTCONbits.TMR0IF = 0;
}

...you can implement your 100Hz contol algorithm.

-Kevin

Ahh. I understand. No ISR at all. That would make it run slightly slower than 100hz, but will probably help performance slightly (due to the lack of the ISR). Thank you very much.

EDIT: Actually, I am not sure if it will run any more slowly at all. This looks great.

EDIT 2: Timer 0 does not seem to have the support to run to a predetermined value like timers 2,3,4. Should we use timer 2 instead? Another question. None of these timers actually allow us to run a 100hz cycle, even with 1:16 pre- and post-scalers. The slowest we can do is 150hz. Am I missing a way to do it, without using a second flag, or should we just use 150hz (or a double flag)?

EDIT 3: I just ran into another potential obstacle. Our calculations say that we will get at most 15 ticks from the encoder per cycle. That does not seem like a very high resolution. We're currently trying to see if this will actually be an issue, and if so, what we could do about it.

spack
14-01-2008, 22:26
I see the linker problem when enabling some but not all of the encoders in the 3 to 6 range.

Chad

spack
14-01-2008, 22:42
The ISR linker issue occurs if encoders_3 and beyond are enabled because...

ENABLE_INT_3_6 must be defined in ifi_frc.h, in order to use encoders in the range 3_6

This causes 'calls' to the ISR routines from ifi_frc.c lines 326 to 338.

But if this code block is enabled, all the interrupt code must exists in encoder.c.

That only happens if ENABLE_ENCODER_3 through ENABLE_ENCODER_6 are ALL defined in encoder.h which compiles in ALL the ISRs in encoder.c.

The define hierarchy needs to be fixed, or use all of 3_6 ISRs or none of them.

Chad

Kevin Watson
15-01-2008, 01:43
The define hierarchy needs to be fixed, or use all of 3_6 ISRs or none of them.Yep, it doesn't quite work correctly. Thanks for catching my gaff. I'll have an update probably tomorrow.

-Kevin

billbo911
15-01-2008, 14:40
Yep, it doesn't quite work correctly. Thanks for catching my gaff. I'll have an update probably tomorrow.

-Kevin

Kevin,
First I would like to say, Thank you in advance.


Is it possible that once you make this modification to the project files and post them, that you could also list the files that were modified so we would be able to just copy the modified files into our already customized projects and then just rebuild. It would save quite a bit of time if we didn't have to make all the customizations again. If not, no sweat, we could always use the practice. :cool:

Kevin Watson
15-01-2008, 16:26
Does it give anyone heartburn if I just release one build for each of the two compiler versions that has support for the ADC, gyro and encoders already built in? You'd be able to remove funtionality by commenting out a few #defines.

-Kevin

bronxbomber92
15-01-2008, 16:28
I have no problem with that.

billbo911
15-01-2008, 16:40
Does it give anyone heartburn if I just release one build for each of the two compiler versions that has support for the ADC, gyro and encoders already built in? You'd be able to remove functionality by commenting out a few #defines.

-Kevin

IMHO, that would be ideal!!!

I don't mind hacking together various functions, in fact, I have learned a lot by doing this in the last couple years. But, if it is together already, it would be VERY convenient! ;)

michelita2607
15-01-2008, 17:09
I know this might be kinda random, but does anyone know if we are allowed to use any kind of gps system in our robot sense it's location on the field?

kaszeta
15-01-2008, 17:36
Does it give anyone heartburn if I just release one build for each of the two compiler versions that has support for the ADC, gyro and encoders already built in? You'd be able to remove funtionality by commenting out a few #defines.

-Kevin

This seems like a more efficient approach for both you and people using the code.

d235j
15-01-2008, 19:37
I know this might be kinda random, but does anyone know if we are allowed to use any kind of gps system in our robot sense it's location on the field?
This post is irrelevant to this topic, but in any case GPS is too inaccurate for this use. Also, it involves receiving external communications, which I understand is against the rules.

Jon236
15-01-2008, 21:43
Does it give anyone heartburn if I just release one build for each of the two compiler versions that has support for the ADC, gyro and encoders already built in? You'd be able to remove funtionality by commenting out a few #defines.

-Kevin

Excellent idea!

BTW, would you think that using 4 interrupts,INT's 3-6, at 128 ints/sec (every ~8ms) would be overstressing the processor? My students want to control our mecanum drive with an encoder on each wheel; I favor the gyro approach.

Thanks again for your work for the teams!

comphappy
16-01-2008, 00:04
How are people implementing these updates?

Joe Ross
16-01-2008, 04:38
Does it give anyone heartburn if I just release one build for each of the two compiler versions that has support for the ADC, gyro and encoders already built in? You'd be able to remove funtionality by commenting out a few #defines.

-Kevin

We would definitely prefer it that way.

spack
16-01-2008, 08:36
Does it give anyone heartburn if I just release one build for each of the two compiler versions that has support for the ADC, gyro and encoders already built in? You'd be able to remove funtionality by commenting out a few #defines.

-Kevin

That sounds good. Doing code management through a one way CVS or some such code repository arrangement where our work can be locally merged but we can stay up to date with you would be nice :)

Chad

Nathans
16-01-2008, 11:03
We're testing the new gyro code, but it overrides all of our commands and sets our pwm outs to 0. Once we put anything referencing the pwm outs we're using, it messes them up. We searched through the code, but we can't find anything that might be causing this. Has anyone else come across this problem?

Kevin Watson
16-01-2008, 17:10
We're testing the new gyro code, but it overrides all of our commands and sets our pwm outs to 0. Once we put anything referencing the pwm outs we're using, it messes them up. We searched through the code, but we can't find anything that might be causing this. Has anyone else come across this problem?Assuming you're referring to my code, what makes you think the gyro code is causing this? Can you use the "Find in files" search tool to find all instances of the PWM variable that's getting fragged?

-Kevin

Nathans
16-01-2008, 17:36
Assuming you're referring to my code, what makes you think the gyro code is causing this? Can you use the "Find in files" search tool to find all instances of the PWM variable that's getting fragged?

-Kevin

We tried that on Monday. I don't have the results in front of me now, but they were all just definitions. I suspect it has something to do with the gyro code (as downloaded from this thread, not added in by us), as the plain C18 3.0 code doesn't cause this problem. We got this bug on 2 occasions, each time starting from scratch, adding only a few lines for straight joystick-to-PWM drive code, as well as removing the gyro calibration in one instance.

Lafleur
16-01-2008, 18:27
Kevin:

Is there a hard limit at 128 bytes in the Serial Queue's??

RX_1_QUEUE_SIZE 128 // Must be a power of two (i.e.,8,16,32,64,128)

ay2b
16-01-2008, 19:02
(This may be a little bit of a tangent.)

I'd like to report to anyone who's interested that the C18 3.10 compiler works just fine under Wine (on Linux), after fixing one minor issue.

I had v 2.4 working under Wine. I ran the upgrade, and then things did not work. I don't remember the exact error, but it did not compile successfully. I copied my mcc18 directory from one machine (where I'd run the upgrade) to a different machine, and it worked fine on the new machine. I then copied my ~/.wine directory from the new machine back to the old machine, and the compiler worked fine.

If you plan on using v 3.10 under Wine, my recommendation is to do one of the following:
- Run the upgrade on a Windows machine, and then copy over the mcc18 directory to run it under Wine
- Make a backup of ~/.wine, then run the upgrade program under Wine, then restore ~/.wine.

Kevin Watson
16-01-2008, 20:44
Kevin:

Is there a hard limit at 128 bytes in the Serial Queue's??

RX_1_QUEUE_SIZE 128 // Must be a power of two (i.e.,8,16,32,64,128)Unless you want to hack the linker script, I suspect the upper limit would be 256. Just out of curiosity, why do you need such a large buffer?

-Kevin

Kevin Watson
16-01-2008, 20:51
We tried that on Monday. I don't have the results in front of me now, but they were all just definitions. I suspect it has something to do with the gyro code (as downloaded from this thread, not added in by us), as the plain C18 3.0 code doesn't cause this problem. We got this bug on 2 occasions, each time starting from scratch, adding only a few lines for straight joystick-to-PWM drive code, as well as removing the gyro calibration in one instance.I can't think of anything in the gyro or ADC code that would cause this, but I'd be very interested in finding out if it's something knuckle-headed I'm doing in my code. Which compiler are you using and which robot controller are you using (i.e., what year is it from)? Can you zip up your code and send it to me?

-Kevin

Lafleur
17-01-2008, 10:54
Unless you want to hack the linker script, I suspect the upper limit would be 256. Just out of curiosity, why do you need such a large buffer?

-Kevin

I don't, just wanting to know the limits of your code...

thanks...

PhilBot
17-01-2008, 14:41
Does it give anyone heartburn if I just release one build for each of the two compiler versions that has support for the ADC, gyro and encoders already built in? You'd be able to remove funtionality by commenting out a few #defines.

-Kevin

I vote for the "All in one" solution..... saves me from having to combine them (which I have a high probability of screwing up).

Phil.

billbo911
17-01-2008, 14:45
Kevin,
This is just a thought and should be implemented by teams as they feel their needs require.
In the teleop.c file, you currently have the gyro bias calculation process happen in the Teleop function.
void Teleop(void)
{
static unsigned int i = 0;
static unsigned int j = 0;
int temp_gyro_rate;
long temp_gyro_angle;
int temp_gyro_bias;

i++;
j++; // this will rollover every ~1000 seconds

if(j == 10)
{
printf("\rCalculating Gyro Bias...");
}

if(j == 60)
{
// start a gyro bias calculation
Start_Gyro_Bias_Calc();
}

if(j == 300)
{
// terminate the gyro bias calculation
Stop_Gyro_Bias_Calc();

// reset the gyro heading angle
Reset_Gyro_Angle();

printf("Done\r");
}


if(i >= 30 && j >= 300)
{
temp_gyro_bias = Get_Gyro_Bias();
temp_gyro_rate = Get_Gyro_Rate();
temp_gyro_angle = Get_Gyro_Angle();
printf(" Gyro Bias=%d\r\n", temp_gyro_bias);
printf(" Gyro Rate=%d\r\n", temp_gyro_rate);
printf("Gyro Angle=%d\r\n\r\n", (int)temp_gyro_angle);
i = 0;
}

Update_OI_LEDs(); // located in ifi_code.c
}


This may be fine in some instances, but in competition, the robot is powered up in disabled mode, then transitions into autonomous(Hybrid) then back to disabled and finally into teleop. The gyro bias would never be calculated until well after it may be needed.
Wouldn't it be better if this code were placed in the disabled.c file in the Disabled function. An additional flag could be set to prevent the bias calc process from running during any additional disabled periods.

Kevin Watson
17-01-2008, 17:16
We're testing the new gyro code, but it overrides all of our commands and sets our pwm outs to 0. Once we put anything referencing the pwm outs we're using, it messes them up. We searched through the code, but we can't find anything that might be causing this. Has anyone else come across this problem?I had a look at your code and it looks like the problem is your limit() function, which is returning a 16-bit integer when it should return an unsigned 8-bit byte. You also had semicolons immediatly after your if statments:


int limit(int value)
{
if(value >255);
value =255;
if(value <0);
value =0; <== because of the added semicolon above, this statement will always execute and will always set the return value to zero.
return value;
}



Your code should probably look like this:


unsigned char limit(int value)
{
if(value >255);
value =255;
else if(value <0);
value =0;
return (unsigned char)value;
}


-Kevin

Kevin Watson
17-01-2008, 17:22
Wouldn't it be better if this code were placed in the disabled.c file in the Disabled function. An additional flag could be set to prevent the bias calc process from running during any additional disabled periods.I've already made this change and provided a sample calibration routine in Disabled(). You're waiting because I'm deep in procrastination mode and haven't finished the first cut of the documentation yet <grin>.

-Kevin

Dave Scheck
17-01-2008, 17:28
Your code should probably look like this:

unsigned char limit(int value)
{
if(value >255);
value =255;
else if(value <0);
value =0;
return (unsigned char)value;
}
Kevin, I think you forgot to remove the semicolons after the IFs in your reply. I think you meant it to beunsigned char limit(int value)
{
if(value > 255)
value = 255;
else if(value < 0)
value = 0;
return (unsigned char)value;
}

Nathans
17-01-2008, 17:40
Thank you. I thought it was probably something stupid I did.

Kevin Watson
17-01-2008, 18:00
Kevin, I think you forgot to remove the semicolons after the IFs in your reply. I think you meant it to beunsigned char limit(int value)
{
if(value > 255)
value = 255;
else if(value < 0)
value = 0;
return (unsigned char)value;
}

Ahh, my devious little plan succeeded and I verified that at least one person is awake and actually reading the programming forum.

Dave, thanks for catching my gaff <grin>.

-Kevin

kaszeta
17-01-2008, 20:26
I had a look at your code and it looks like the problem is your limit() function, which is returning a 16-bit integer when it should return an unsigned 8-bit byte.

This is probably one of the big things to watch for, even for experienced programmers. The MPLAB/PIC18 platform (and embedded platforms in general) are a lot less tolerant of type mismatches than your usual programming.

billbo911
17-01-2008, 21:46
You're waiting because I'm deep in procrastination mode and haven't finished the first cut of the documentation yet <grin>.
-Kevin

I think someone needs to pull a high priority interrupt and put everything else you are doing on hold! This should be your top priority, cuz I said so.:p

"Procrastination mode" I wonder if FIRST will use that to replace Hybrid mode next year?

lukevanoort
17-01-2008, 22:28
"Procrastination mode" I wonder if FIRST will use that to replace Hybrid mode next year? Well, in the past, the difference wouldn't be very noticeable...

Kevin Watson
17-01-2008, 23:46
BTW, would you think that using 4 interrupts,INT's 3-6, at 128 ints/sec (every ~8ms) would be overstressing the processor?I depends on what else is executing on the processor, but in general you shouldn't have a problem.

-Kevin

Kevin Watson
18-01-2008, 01:55
I think someone needs to pull a high priority interrupt and put everything else you are doing on hold! This should be your top priority, cuz I said so.:p Okay, here is a snapshot of the current build that includes support for encoders and gyro:

http://kevin.org/frc/ifi_frc_beta_4.zip

*Please* help me out and have a look at the code and documentation (start with readme.txt) and provide feedback before I go public with the code on my website, which I'd like to do on Friday or early Saturday. If you can, follow the directions in readme.txt to build the code and test with your encoder(s) and/or gyro and report any problems here. Thanks.

-Kevin


Edit: Code has been released and is available here: http://kevin.org/frc.

-Kevin

Mike Mahar
18-01-2008, 11:05
Kevin,
In your gyro initialization code in teleop.c, you have two static ints that you use as counters. (i, j) You increment both every time through the function. When you wish to print out the gyro you have the following:

if(i == 35 && j >= 300)


Since i has been incrementing all along, it will be equal to j. The next time i == 35 j will also be equal to 35. Putting i = 0 into the if(j == 300) body should fix that. You also need to put i = 0 into the body of the if(==35 && i >= 300) statement if you aren't using the encoder code.

Kevin Watson
18-01-2008, 11:58
Kevin,
In your gyro initialization code in teleop.c, you have two static ints that you use as counters. (i, j) You increment both every time through the function. When you wish to print out the gyro you have the following:

if(i == 35 && j >= 300)


Since i has been incrementing all along, it will be equal to j. The next time i == 35 j will also be equal to 35. Putting i = 0 into the if(j == 300) body should fix that. You also need to put i = 0 into the body of the if(==35 && i >= 300) statement if you aren't using the encoder code.Thanks, I'll have a look to see if I can clean it up a bit.

-Kevin

billbo911
18-01-2008, 16:07
In the README.TXT, in the section to enable the ISR's in ifi_frc.h, (bullet # 3)the description is misleading, or the file is incorrect.

3) Enable the interrupt service routines associated with each encoder
channel at the top of ifi_frc.h.

When looking at the file, the lines do not indicate that you are enabling an ISR.
// #define ENABLE_INT_1 // enable if using encoder channel 1
// #define ENABLE_INT_2 // enable if using encoder channel 2
// #define ENABLE_INT_3 // enable if using encoder channel 3...

Documentation error, code error, reader misunderstanding error???

Kevin Watson
18-01-2008, 16:34
Documentation error, code error, reader misunderstanding error???Given the comments above and alongside those #defines, I don't see how a user wouldn't be able to figure it out:


// Remove the comment slashes from one or more of the following lines to
// enable the respective external interrupt(s) and/or timer(s). By doing
// so, you only enable the code within ifi_frc to become part of your
// software build.
// #define ENABLE_INT_1 // enable if using encoder channel 1
// #define ENABLE_INT_2 // enable if using encoder channel 2
// #define ENABLE_INT_3 // enable if using encoder channel 3
// #define ENABLE_INT_4 // enable if using encoder channel 4
// #define ENABLE_INT_5 // enable if using encoder channel 5
// #define ENABLE_INT_6 // enable if using encoder channel 6


I'd be happy to change it, but I don't fully understand the problem...

-Kevin

billbo911
18-01-2008, 16:46
I just saw the answer in encoder.c.
Sorry for the confusion. My bad.:mad:
I hate it when I misinterpret instructions like that. Oh well, blame it on sleep deprivation.;)