Notice

The forum is in read only mode.

General development guidelines

PhracturedBlue
Topic Author
Offline

25 Apr 2012 21:16 #49 by PhracturedBlue

Replied by PhracturedBlue on topic Re: General development guidelines

I'll check it out once I get home, but I don't see an issue in the code. The memcpy is being compiled as:

 8009918:       e1a0c001        mov     ip, r1
 ...
 80099c8:       e1a0100c        mov     r1, ip
 80099cc:       e4d1c001        ldrb    ip, [r1], #1
 80099d0:       e2522001        subs    r2, r2, #1
 80099d4:       e4c3c001        strb    ip, [r3], #1
 80099d8:       1afffffb        bne     80099cc <memcpy+0xc0>

which should work fine
Did you try running in the debugger? I found that using gdb with the STLink and loading my elf file works reasonably well, even if it is a little fragile.

PhracturedBlue
Topic Author
Offline

26 Apr 2012 15:30 #61 by PhracturedBlue

Replied by PhracturedBlue on topic Re: General development guidelines

I indeed see that the memcpy hangs the MCU (and that it is the memcpy and not something about printf). Sorry, I ran out of time before I could load it into the debugger and step through it. Maybe something about reading directly from ROM? It doesn't make much sense to me.

rcH4x0r
Offline

26 Apr 2012 16:09 #62 by rcH4x0r

Replied by rcH4x0r on topic Re: General development guidelines

-Moving the tmp array off the stack doesn't help.
-A memset to fill the buffer with 'A' hangs too.
-A simple for loop to copy the 7 bytes from flash to the buffer works (as does printf).

Looks like there is an issue with the libs/toolchain

PhracturedBlue
Topic Author
Offline

26 Apr 2012 16:46 #63 by PhracturedBlue

Replied by PhracturedBlue on topic Re: General development guidelines

I think there s something wrong with the branch:

 8006a78:       f002 ef48       blx     800990c <memcpy>

(That isn't actually a blx, as the first nibble is 'f' which marks it as a 'BL' instruction (which makes more sense, so I guess objdump is confused see below)
However, the 1st nibble in the second byte is 'e' which is not valid for 'BL' in thumb or thumb2 as far as I can tell (it should be 11x1)

So the compiler seems to be generating bad code which is probably what confused objdump. This is the only occurrence of this I see in the firmware, so maybe something about newlib?

PhracturedBlue
Topic Author
Offline

26 Apr 2012 19:04 #66 by PhracturedBlue

Replied by PhracturedBlue on topic Re: General development guidelines

I'm not at home at the moment to test it, but try the following:
in Makefile change:

$(TARGET).elf: $(LINKFILE) $(OBJS)
        $(CC) -o $@ $(OBJS) $(LFLAGS)

$(TARGET).elf: $(LINKFILE) $(OBJS)
        $(CC) -o $@ $(OBJS) $(LFLAGS) $(CFLAGS)

I did this, and the compiled code now looks like this:

 8006a78:       f002 ff48       bl      800990c <memcpy>

which looks a lot better

rcH4x0r
Offline

26 Apr 2012 20:25 #70 by rcH4x0r

Replied by rcH4x0r on topic Re: General development guidelines

Didn't work for me. I am busy this weekend, unfortunately, so I cannot help much more

PhracturedBlue
Topic Author
Offline

27 Apr 2012 02:27 #71 by PhracturedBlue

Replied by PhracturedBlue on topic Re: General development guidelines

Well, the changes I made to the Makefile and just committed (the above plus one other addition) do seem to have solved the issue for me. I no longer get hangs with the memcpy with trunk. If it doesn't work for you, send me you .list file and I'll take a look.

rcH4x0r
Offline

27 Apr 2012 04:49 - 27 Apr 2012 20:14 #72 by rcH4x0r

Replied by rcH4x0r on topic Re: General development guidelines

Hopefully the "one other" will fix it for me too. When I get time I will test again

Edit: Yes, that fixed it, onwards!

Last edit: 27 Apr 2012 20:14 by rcH4x0r.

wuselfuzz
Offline

01 Aug 2012 15:25 - 01 Aug 2012 15:26 #805 by wuselfuzz

Replied by wuselfuzz on topic Re: General development guidelines

My two cents:

About using C++:

I tried an object oriented C++ approach for my quadcopter stuff once which resulted in a PWM interface with a Sam7xPWM implementation accessible as a singleton.

The resulting user code was about this simple:

int main() {
  PWM& pwm=Sam7xPWM::getInstance();
  pwm[0]=1000;
  pwm[1]=500;
}

All the neccessary hardware setup (configuring GPIOs, creating a microsecond timer, setting up all four sam7x hardware pwm channels) worked automagically in the singleton's static constructor.

Even cout worked through the USART.

However, the drawbacks were:

- IMO, it's nicer to have a hardware_init()-function called at the start of main() that sets everything up in a specific order. Instead of read-modify-write operations for e.g. the data direction registers of a port belonging to a peripheral, this allows you to set up the registers once and be done with that.

- Using cout just once added many kilobytes of additional code. Much more bloat than just using printf.

- You need a more complex linker script and startup code to properly call static constructors.

About global (or rather file scope) variable use:

I'd rather use an object-oriented C (NOT C++) approach at many places and avoid global variables. As an example, there's the global struct Model Model and there's the function

u8 MIXER_MapChannel(u8 channel).

In an OO approach, Model would be the object, mode would be a property and MapChannel would be a method. "object oriented C" would then be like

u8 MODEL_MapChannel(struct Model *m, u8 channel) {
   switch(m->mode) {
      ...
   }
   ...
}

Why all the hassle? Think about flight mode switches, dual rates, and the likes.

Last edit: 01 Aug 2012 15:26 by wuselfuzz.

PhracturedBlue
Topic Author
Offline

01 Aug 2012 16:14 #806 by PhracturedBlue

Replied by PhracturedBlue on topic Re: General development guidelines

wuselfuzz wrote:
About global (or rather file scope) variable use:

I'd rather use an object-oriented C (NOT C++) approach at many places and avoid global variables. As an example, there's the global struct Model Model and there's the function

u8 MIXER_MapChannel(u8 channel).

In an OO approach, Model would be the object, mode would be a property and MapChannel would be a method. "object oriented C" would then be like
u8 MODEL_MapChannel(struct Model *m, u8 channel) {
   switch(m->mode) {
      ...
   }
   ...
}

If you notice, I am not very consistent about this. In some cases I pass the object, and in others I directly access global variables. From a uC perspective I am used to programming on more limited platforms than the STM32, and there can be significant overhead to passing more than a couple parameters in a function call. That probably isn't particularly relevant to the stm32 though. In any case, I'm open to any improvements to cleaning up the code.

I'm not sure exactly what you are referring to here though:

Why all the hassle? Think about flight mode switches, dual rates, and the likes.

The mapping is only relevant for the 4 sticks (as all other input are labeled, there is no confusion about what they do). I'm not sure this is the best example to show your point.
It may make more sense for Model to be private to mixer.c and to use accessors to get at it (you'll see some of that in the code as well), but I don't think doing so will have a significant impact on the MapChannels function.

PhracturedBlue
Topic Author
Offline

01 Aug 2012 16:15 #807 by PhracturedBlue

Replied by PhracturedBlue on topic Re: General development guidelines

Also, if you do want to make changes like this (which is fine) I'd like to see the checkins done incrementally rather than wholesale so we don't disrupt the code base too significnatly.

FDR
Offline

01 Aug 2012 16:40 #808 by FDR

Replied by FDR on topic Re: General development guidelines

I guess a better example would be the control structure and it's event handling...
That could benefit from inheritance too...

Time to create page: 0.102 seconds

Force desktop display