bipropellant / bipropellant-hoverboard-firmware

OpenSource Hoverboard firmware based on Niklas Fauth's one https://github.com/NiklasFauth/hoverboard-firmware-hack
GNU General Public License v3.0
176 stars 75 forks source link

please fix Makefile #59

Open RoboDurden opened 5 years ago

RoboDurden commented 5 years ago

would add this repo to my online compiler www.pionierland.de/hoverhack

errors so far:

Now i get

inc/protocolfunctions.h:8:8: error: unknown type name 'PROTOCOL_STAT'
 extern PROTOCOL_STAT sSoftwareSerial;

There is no PROTOCOL_STAT defined anywhere into the repo.

p-h-a-i-l commented 5 years ago

did you clone with submodules? https://github.com/bipropellant/bipropellant-hoverboard-firmware#building-and-flashing

RoboDurden commented 5 years ago

my online compile simply downloads https://github.com/bipropellant/bipropellant-hoverboard-firmware/archive/master.zip Then unzips it, replaces config.h and main.c and then runs make. Works for https://github.com/p-h-a-i-l/hoverboard-firmware-hack but fails here.

Please a link to the zip file of the missing stuff :-)

p-h-a-i-l commented 5 years ago

Works for https://github.com/p-h-a-i-l/hoverboard-firmware-hack but fails here.

The main branch in that fork is based on an older protocol revision which does not use submodules yet.

Have a look at the source folder, you will find that src/hbrotocol links to a commit in https://github.com/bipropellant/bipropellant-protocol. Although it will probably work most of the times, I'd advise against downloading the .zip of the master branch of submodules. In our repository, we specify a single commit. That is the revision of the submodule which was intended to be included by the maintainer. There might be newer versions in the submodule repo which are not yet compatible to the main repo.

Why don't you use git to grab the code? That would be a single command.

RoboDurden commented 5 years ago

Okay i downloaded https://github.com/bipropellant/bipropellant-protocol into my inc/ and src/ folders and now it compiles successfully.

Why don't you use git to grab the code? That would be a single command.

I guess i first would have to install git on my vserver ?? Then what would be the single command ?

p-h-a-i-l commented 5 years ago

first time: git clone --recurse-submodules https://github.com/bipropellant/bipropellant-hoverboard-firmware.git afterwards git pull

RoboDurden commented 5 years ago

yes i installed git on my vserver :-/ now all three repos compile successfully.

but i only check the latest-commit date in the html page. Your submodule gives a fatal error when checking via git: `git status

fatal: Not a git repository: /var/www/html/hoverhack/source/bipropellant-hoverboard-firmware/.git/modules/lib/hbprotocol `

already half a day wasted on this linux shit :-(

p-h-a-i-l commented 5 years ago

try reinitialising the submodule..

cd /var/www/html/hoverhack/source/bipropellant-hoverboard-firmware
git submodule update --init --recursive 
cd src/hbprotocol
git status

already half a day wasted on this linux shit :-(

frustration is part of learning :) I wasted so many ours on stupid things around programming and electronics, but in the end I still have fun.

RoboDurden commented 5 years ago

same error:

root@vps:/var/www/html/hoverhack/source/bipropellant_bipropellant-hoverboard-fir
mware# git submodule update --init --recursive

fatal: Not a git repository: /var/www/html/hoverhack/source/bipropellant-hoverboard-firmware/.git/modules/lib/hbprotocol

Unable to find current revision in submodule path 'src/hbprotocol'

but i will try to avoid this new fork anyway.. When i see how awfully complicated your new usar2 protocol has become..

frustration is part of learning :)

No definitely NOT !

This again confirms my strong dislike for the linux community who still have not heard of oop. I would try to replace the gcc with g++ in the Makefile. Then an abstract class CModule and and array CModule aModule[10] And all the different Controls in their own //#include "CModuleAdc.h" which simply push themselves onto aModule. And in the main.c simply loops like for (int i=0; i<iCountModule; i++) aModule[i].Init(); and for (int i=0; i<iCountModule; i++) aModule[i].Tick();

Then you could easily add your new CControlUSART and CReadSpeed to the originial repo ..

That would be joy !

roland the little physicist and https://www.youtube.com/watch?v=15UBBcZvMh8

p-h-a-i-l commented 5 years ago

same error:

root@vps:/var/www/html/hoverhack/source/bipropellant_bipropellant-hoverboard-fir
mware# git submodule update --init --recursive

fatal: Not a git repository: /var/www/html/hoverhack/source/bipropellant-hoverboard-firmware/.git/modules/lib/hbprotocol

Unable to find current revision in submodule path 'src/hbprotocol'

I'd delete the directory and start with a clean clone now. But I don't think you're willing to try further. You can go with your .zip solution and risk inconsistence, but It'll probbably work most of the times.

but i will try to avoid this new fork anyway.. When i see how awfully complicated your new usar2 protocol has become..

It has grown out of need. if you have a simple solution, feel free to contribute!

This again confirms my strong dislike for the linux community who still have not heard of oop.

We are neither the linux community nor do I think this claim holds true..

I would try to replace the gcc with g++ in the Makefile. Then an abstract class CModule and and array CModule aModule[10] And all the different Controls in their own //#include "CModuleAdc.h" which simply push themselves onto aModule. And in the main.c simply loops like for (int i=0; i<iCountModule; i++) aModule[i].Init(); and for (int i=0; i<iCountModule; i++) aModule[i].Tick();

Then you could easily add your new CControlUSART and CReadSpeed to the originial repo ..

I do also prefer to use C++ with microcontrollers, but with motor control you sometimes C is more suited. Also we are building on top of existing code, there was no need to refactor to C++ yet. But your contribution is welcome, if you're willing to invest some time.

RoboDurden commented 5 years ago

i am happy that you respond so quickly ! So yes i will try again to make your repo fully available in my online compiler. I am currently debuging a c++ Makefile of the original source. Looking good so far.

btsimonh commented 5 years ago

one of the issues I found when playing with a C++ class in this was that immediately the code size doubled; it did not seem to only include the libraries I wanted... :(. We are not short on space, but nice to keep it fairly slim.

RoboDurden commented 5 years ago

all the #ifdef CONTROL_ throughout the source code is simply horrific. And especially for a github project where multiple people want to contribute, the first fork of Niklas his repo should have been a c++ port.

p-h-a-i-l commented 5 years ago

all the #ifdef CONTROL_ throughout the source code is simply horrific. And especially for a github project where multiple people want to contribute, the first fork of Niklas his repo should have been a c++ port.

Agreed, my goal is to have one binary which can be configured at runtime.

RoboDurden commented 5 years ago

This is a typical Linux response ;-) As long as the final binary runs, you do not care how awful the source code is. And that is why frustration belongs to your everyday life. oop instead is pure beauty :-)

AntumArk commented 5 years ago

Well we do care how our source code is. We are working with fairly limited hardware and we need to squeeze everything we can. @RoboDurden What kind of safety features do you want for protocol to have? (Checksums, CRC etc)

RoboDurden commented 5 years ago

Right now i would like an Arduino .ino to test your USART2_CONTROLLED binary The 32bit CRC from https://github.com/p-h-a-i-l/hoverboard-firmware-hack was fine already.

Have not looked into how you send back speed, temperature, battery_volate, currentL and currentR via USART2

But when you have received a command then you should simply immediately return such a struct containing a crc32

So when i send from the esp8266 every 250 ms i will always get a response with the state of the hoverboard controller :-)

RoboDurden commented 5 years ago

c++ port.. after only minor changes i am stuck at

arm-none-eabi-g++ build/stm32f1xx_hal_flash.o build/stm32f1xx_hal_pwr.o build/stm32f1xx_hal_rcc.o build/stm32f1xx_hal_tim.o build/stm32f1xx_hal_tim_ex.o build/stm32f1xx_hal_gpio_ex.o build/stm32f1xx_hal_adc_ex.o build/stm32f1xx_hal_cortex.o build/stm32f1xx_hal_flash_ex.o build/stm32f1xx_hal_gpio.o build/stm32f1xx_hal_rcc_ex.o build/stm32f1xx_hal.o build/stm32f1xx_hal_adc.o build/stm32f1xx_hal_uart.o build/stm32f1xx_hal_i2c.o build/stm32f1xx_hal_dma.o build/system_stm32f1xx.o build/setup.o build/control.o build/main.o build/bldc.o build/comms.o build/stm32f1xx_it.o build/startup_stm32f103xe.o -mcpu=cortex-m3 -mthumb   -specs=nano.specs -TSTM32F103RCTx_FLASH.ld  -lc -lm -lnosys -Wl,-Map=build/hover.map,--cref -Wl,--gc-sections -o build/hover.elf

/usr/bin/../lib/gcc/arm-none-eabi/7.3.1/../../../../arm-none-eabi/lib/thumb/v7-m/libc_nano.a(lib_a-abort.o): In function `abort':

abort.c:(.text.abort+0xa): undefined reference to `_exit'

/usr/bin/../lib/gcc/arm-none-eabi/7.3.1/../../../../arm-none-eabi/lib/thumb/v7-m/libc_nano.a(lib_a-signalr.o): In function `_kill_r':

signalr.c:(.text._kill_r+0xe): undefined reference to `_kill'

/usr/bin/../lib/gcc/arm-none-eabi/7.3.1/../../../../arm-none-eabi/lib/thumb/v7-m/libc_nano.a(lib_a-signalr.o): In function `_getpid_r':

signalr.c:(.text._getpid_r+0x0): undefined reference to `_getpid'

collect2: error: ld returned 1 exit status

Makefile:145: recipe for target 'build/hover.elf' failed

Where are these functions abort , _kill_r , _getpid_r located ? am afraid that the stm32 libraries are not c++ compatible ?

AntumArk commented 5 years ago

Right now i would like an Arduino .ino to test your USART2_CONTROLLED binary>

Check Readme.md

Have not looked into how you send back speed, temperature, battery_volate, currentL and currentR via USART2>

You should send same packet as writing , but instead of 'W', send 'R'. Then you should expect full structure sent back. For current measurments, use 0x08 command.

But when you have received a command then you should simply immediately return such a struct containing a crc32>

I do agree that using CRC would be better and faster result than just waiting for ACks or NACks. But I am currently working on a different part of a project, so I won't be able to change that for quite a while.

p-h-a-i-l commented 5 years ago

But when you have received a command then you should simply immediately return such a struct containing a crc32>

I do agree that using CRC would be better and faster result than just waiting for ACks or NACks. But I am currently working on a different part of a project, so I won't be able to change that for quite a while.

So you call for more simplicity but want to replace the simple checksum we are now using with a crc.

Checksums and ACKs have very different purpose. Checksums indicate to the RECEIVER if the data is corrupt, ACKs indicate to se SENDER if data was received. They are not interchangeable. Even an ACK carries a checksum.

The ACKs and NACKs are optional, you can choose to use them or not. I use them for things like configuration parameters. This way the protocol takes care of resending when communication is disturbed. For control values like pwm and measurements like speed I do prefer working without ACKs. No need to resend, the next package will be sent soon..

RoboDurden commented 5 years ago

c++ port.. Okay i successfully compiled the source of the original repo :-) But have not flashed and tested it.

changes in Makefile

LIBS = -lc -lc -lrdimon -lnosys #ROBO

CC = $(PREFIX)g++

AS = $(PREFIX)g++ -x assembler-with-cpp

main.c

int abs(int i)  //ROBO
{
    if (i<0) return -i;
    return i;
};

and three times HAL_GPIO_WritePin(OFF_PORT, OFF_PIN,(GPIO_PinState) 0); //ROBO

setup.c

extern UART_HandleTypeDef huart2;   //ROBO
extern DMA_HandleTypeDef hdma_i2c2_rx;  //ROBO
extern DMA_HandleTypeDef hdma_i2c2_tx;  //ROBO

control.c

extern DMA_HandleTypeDef hdma_i2c2_rx;  //ROBO
extern DMA_HandleTypeDef hdma_i2c2_tx;  //ROBO

comms.c

extern UART_HandleTypeDef huart2;   //ROBO

char uart_buf[100]; //ROBO volatile 
int16_t ch_buf[8];  //ROBO volatile 

setup.h

void setScopeChannel(uint8_t ch, int16_t val);
void consoleScope();

So you call for more simplicity but want to replace the simple checksum we are now using with a crc.

I don't need an ACK when i send new commands every 250ms. But what would really help me now is a .ino example o c-example on how to communicate via USART2

With your complicated protocol i fear that it will take me hours to write my own code. That is why i still think to continue with the old repo of @p-h-a-i-l . I can easily rewrite the CRC function to run on data[i+iOffset%8] . Then i simply loop 8 times over the crc check until i have found a valid crc. With a 32bit crc that will still be 99.9% safe. That would only take me 5 minutes and for me it would be more joy to go on with the c++ port

p-h-a-i-l commented 5 years ago

I don't need an ACK when i send new commands every 250ms.

That's what I wrote.

But what would really help me now is a .ino example o c-example on how to communicate via USART2

again. in the readme.. : https://github.com/bipropellant/bipropellant-hoverboard-firmware/blob/master/README.md#serial-machine-control

That is why i still think to continue with the old repo of @p-h-a-i-l . I can easily rewrite the CRC function to run on data[i+iOffset%8] . Then i simply loop 8 times over the crc check until i have found a valid crc. With a 32bit crc that will still be 99.9% safe. That would only take me 5 minutes and for me it would be more joy to go on with the c++ port

Yep, that's one way to do it. Until you want to read back stuff and set config options etc. How do you plan to get rid of all the #defines without generic communication protocol?

RoboDurden commented 5 years ago

Thank you for the example code. I really have so many things to do that i can not read the full repo.

Yep, that's one way to do it. Until you want to read back stuff and set config options etc.

Whenever i have a valid crc i simply send back a struct with something like void consoleLog(char *message)

How do you plan to get rid of all the #defines without generic communication protocol?

Every CModulControl will implement its own communication "protocol". The CControlAdc will read the two analog inputs, the CControlUART will have its communiction strct, the CCControlTest will have no input :-) All CControl classes will return a command-struc (steer,speed,etc) when CControl::Tick() is called.

A bit more tricky will be CControlUART to access CControlSensor because it needs the actual wheel speed, battery voltage, etc. to send back via its private UART protocol. The CControlSensor could be a "friend" of CControlUART. Or every CModule will have access to the global aModule and can iterate through all the other modules to find the ones they would like to retreive data from or send data to. So of course there could be a CModuleBldc and every CControl can send directly the speed and steer to it. Or there could be a module CModuleMain which first retrieves the speed/steer from a CControl and then sends the final speed/steer to CBldc :-) And every CModule in its own .cpp file and the main.c would only need the wanted #includes.

Then we could all return to the original NiklasFauth repo and there would be all kinds of modules being maintained by their creators.

update: YES, i added this to the main.c and it did compile successfully:


class Point;
class Point {
public:
    int x, y;
    Point (int c1, int c2) { x = c1; y = c2;}
    Point& operator=(Point rhs) {
        x = rhs.x; y = rhs.y;
        return *this;
    }
};

class Complex : public Point {
  private: 
    int &real, &imag;
  public: 
    Complex(int r, int i) : Point (r, i), real (x), imag (y) 
    {
    }
}
```;
btsimonh commented 5 years ago

void setScopeChannel(uint8_t ch, int16_t val); void consoleScope();

yep, the extra lines you have added all relate to stuff has not been used in this repo, so your config.h is not configured for a common configuration used here - you WILL hit trouble. Look through the existing makefile/config.h and replicate a specific, supported configuration.

These bits need to be removed :). especially beware of:

extern UART_HandleTypeDef huart2; //ROBO char uart_buf[100]; //ROBO volatile int16_t ch_buf[8]; //ROBO volatile

If you had to add these, then you've also got some nasty serial DMA in play, and lots of conflicting hardware. I2C is also untested, so the fact you had to add the lines indicates something very wrong.

P.s. Protocols ARE complex. CRC is expensive - we have little power available. These are aspects which were discussed at length previously.

RoboDurden commented 5 years ago

if you had to add these, then you've also got some nasty serial DMA in play, and lots of conflicting

no i only had to add the extern and remove the volatile Don't really know what the extern does and am not really happy to have removed the volatile.

By the way:

 hover_c++.bin 23.296 bytes
 hover_c.bin 18.260 bytes

The flash size of the stm32f103xe is 512 kB :-)

p-h-a-i-l commented 5 years ago

if you had to add these, then you've also got some nasty serial DMA in play, and lots of conflicting

no i only had to add the extern and remove the volatile Don't really know what the extern does and am not really happy to have removed the volatile.

volatile and memcpy don't mix well. There were also some very old compiler warnings about volatile being removed by the compiler. C++ is probably more sctrict about it.

extern tells the compiler that the variable is known and memory is allocated somewhere else. So if it wasn't extern and you made it extern without allocating memory, the linker should cry..

RoboDurden commented 5 years ago

extern tells the compiler that the variable is known and memory is allocated somewhere else. So if it wasn't extern and you made it extern without allocating memory, the linker should cry..

Great you know the details. Yes i got lots of multiple definition errors and already assumed that the extern would tell the linker to stop crying.

volatile and memcpy don't mix well

Well that was only the debug buffer and debug data array. Not much ill done when they get corrupted.

I now have to make a break to relax a bit and then install the c++ firmware.

greetings from Bavaria :-)