Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

EspressLRS support (CRSF decoder) #24

Merged
merged 3 commits into from
Sep 20, 2023
Merged

Conversation

WizzardDr
Copy link

https://cdn.discordapp.com/attachments/596350022191415320/1153628310895599647/VID_20230919_104329.mp4

I added a USART decode mode and made them dependent on USART_CRSF or USART_SERIAL.

@RoboDurden
Copy link
Owner

Nice video, good job.
I can not see the uartSteer connection. Do you have such a tiny ExpressLRS module hidden in the setup ?
boot

Now i have a problem with this pull request :-/
And i would be happy if you could move as much as possible of your CRSF code into a new commsSteeringCRSF.h file.

I really do not like this non-objectOriented #ifdef chaos of the EFeru FOC Gen1 firmware with all the many VARIANT_XY.
Not to say that i hate it.

With my C++ SimpleFOC firmware i go for an abstract base communication (= Pilot) class and begin with a PilotI2c: https://github.com/RoboDurden/Split_Hoverboard_SimpleFOC/blob/main/include/PilotI2c.h

Here with the C firmware, this is not possible.
I think the closed way to a base class and dervied control/pilot classes would be

commsSteering.h containing the TEST_SPEED functionality
commsSteeringUART.h have the current uart code
commsSteeringSRSV.h with your new code

Users already request PWM and ADC control :-(

All three files share the same function definitions so there is no more need for stupid #ifdef TEST_SPEED etc.

The main.c: while(1) loop would call a function CommsSteeringUpdate()
Only the commsSteering.h would then execute the TEST_SPEED code while the other two function implementations would instantly return because they change speed and steer interrupt driven.

If you understand the idea, i would be happy if you would change your code because i can not test it.
But this is not a must.
I am happy for the code you are contributing with this pull request.
So i also could transform your code, push my changes and you update your fork to verify my changes.

Ideas welcome.

@WizzardDr
Copy link
Author

WizzardDr commented Sep 19, 2023

Nope not hidden, middle of the image, there is quite a bit of compression but mainly the receiver module is just tiny!
It is just like the one in the photo.

I think it would help to give your link mode a name since UART is very close to USART... arduinoLink, arduino, command, commandLink

I did look into it but I'm not sure about the intricacies in adding files, hence why I tacked it on.
Feel free to move it around and I can test it.

Making the all three files share the same function definitions means only one can be used, so no mixing and matching adc, uart, pwm for the speed and steer. I do agree with moving the CRSF to a different file, same for adc, pwm, test (treat test like an input source)

Maybe just have some proxy functions in commsSteering.h

#include testInput.h
//#include UARTInput.h
//#include CRSFInput.h
//#include ADCInput.h
//#include RCPWMInput.h (maybe call it RC PWM to not confuse it with real PWM)

DMA calls updateUSARTInput()

updateUSARTInput(){
//UpdateUSARTSteerInput();
//GetCrsfPacket();
}

Main calls updateInput()

updateInput(){
calculateTest();
//readADC();
//readRCPWM();
}

Then main calls updateChannels(); to transfer buffer to channels

updateChannels(){
updateTestChannels();
//updateCRSFChannels();
//updateADCChannels();
//updateRCPWMChannels();
}

In Main.c
In Main you can mix and match between channel data when mixing to create steer, speed, masterSpeed etc.
i.e. speed = testChannel[0];
in main you could then do stuff like speedLimit = rcPwmChannel[0];

@WizzardDr
Copy link
Author

commsSteeringCRSF.h

Could also have proxies for telemetry.

Maybe make a /inputs directory in /src for all the input type files.

@RoboDurden
Copy link
Owner

RoboDurden commented Sep 19, 2023

Okay i will do the design changes. But not today.
There are other users working with different layouts right now so when i accept your pull request i feel the pressure to quickly get your CRSF specific code out of the way.

When if have made the design change to allow different controllers/pilots (always only one so no mixing) it would be great if you make a youtube video tutorial on which parts to buy to use your controll method :-)

What do you think of my idea to name these different controllers "pilots" ?
English is not my native language..

@WizzardDr
Copy link
Author

WizzardDr commented Sep 19, 2023

I am looking at the inputs as devices rather than the person. So using maybe an encoder device via an arduino for a steering wheel while using an adc device input for a throttle, and having the option to connect a pwm rc remote with an override switch for remote control. So I think pilots would be to restrictive of a concept.

I think people's use could cases vary a lot from just a speed and steering input.

For example alongside the remote input I am going to have a autonomous controller loop with a camera so the wheel speed will be controlled 1 to 1 so the kinematics can be calculated for the control.

For the crsf I can make a video, it is quite simple, just a reciver 4 wires and the controller.

What is that aim of abstraction to "controllers", rather than keeping it simply a data stream? InputDevice -> InputValue -> mix/map -> outputPWM -> bldcOutput

@RoboDurden RoboDurden mentioned this pull request Sep 19, 2023
@RoboDurden
Copy link
Owner

Well i really love object orientend programming, one of the most important inventions of last century. More important than the invention of the car or the atomic bomb !
So i always think of objects as persons of a big team who do their job to make the big thing running - like being a pilot.
From the perspective of the hoverboard-board (with the commsMasterSlave), it is rather only about speed left and speed right.
So some "person" is needed to tell the hardware the speedL and speedR. That person would pilot the hardware.
But yes, a tank would need speedL and speedR and a car would need speed and steer.
My log band saw takes distance forward-backward and distance up/down.

But for more complex situations (like vision control), we need an additional ESP32 anyway ?
So again from the perspective of this firmware, the uart communication is the pilot.

Maybe "operator" would also work.
But i do not like these generic words like operator or controller becasue they can mean so many things.
The hardware is already called hoverboard controller :-(

So introducing the concept of a pilot should make it easier to understand where functionality belongs..

@WizzardDr
Copy link
Author

WizzardDr commented Sep 19, 2023

But in the instance of a vehicle like a hoverboard the pilot is the person/machine vision not the interface between them.

The pilot is the thing that tells the Arduino to send a command to the hoverboard, the way that command gets there is just an interface.

Looking at the uart as the pilot would be like limiting a helicopter to thinking the cyclic is the pilot and so it is only able to acknowledge the input of the cyclic (the one and only pilot) disregarding the other "pilots" such as the throttle, pedals, collective.... it wouldn't work.

That is why I am suggesting commsSteering.h as the aggregator, the assemblage of inputs that make up the pilot if you will.

@RoboDurden
Copy link
Owner

Of course, the Arduino code could also have its own Pilot class that implements the vision control and sends speed steer commands to the hoverboard.
But inside the hoverbaord firmware threre would only be the pilot without vision control who gets his idea for speed and steer via uart..

I aggree that there should be a third class in between the uart and the motor: Receiver (uart) -> Pilot (data to speed/steer or speedL/speedR) -> Motors.
Problem is that the receivers protocol already contains the info whether it is speed/steer or speedL/speedR.

So Receiver -> Pilot -> Motor is not that easy to implement.

Problem with "receiver" also is, that commsSteering does not only receive but also transmits data back to the Arduino.

So i prefer to think of that receiver als the pilot and do not care who is piloting the pilot from the Arduino/Esp32/RC-Control/etc.

The pilot is the one who comes up with the speed/steer values.

@RoboDurden
Copy link
Owner

RoboDurden commented Sep 19, 2023

My problem is that today i also succeeded with the I2C implementation in the SimpleFOC firmware: Candas1/Split_Hoverboard_SimpleFOC#11 (comment)
By tomorrow, this Gen2.x firmware here might already be obsolete.
At least for the 64kB MCUs (yours included)
That is why i did not want to offer additional receivers.
But now you have already written a nice one, so i have think how to incooperate a c++ conecpt into this c firmware :-/

@WizzardDr
Copy link
Author

WizzardDr commented Sep 19, 2023

What hardware does simpleFoc need that isn't the 64kb MCU?

I have got some simpleFOCMini that I am yet to use on a fast processor, I have made a custom foc controller / sine closed loop controller with gimbal motor and 3half bridges but it needs more voltage to overcome cogging. Hopefully soon I too will have a good idea on how simpleFOC works, through that project.

I really don't think it needs to be c++ due to the lack of instancing.

@RoboDurden
Copy link
Owner

The layouts with a GD32130C6 MCU only have 32 kB. Our SimpleFOC firmware currently needs 43764 bytes :-(

@WizzardDr
Copy link
Author

WizzardDr commented Sep 19, 2023

Oh, I misread, you mean the foc firmware will run on 64kb... so the Gen2.x will have no use.

@WizzardDr
Copy link
Author

I look forward to testing it... hopefully it should help tight the path following control loop I intend to get working.

@RoboDurden RoboDurden merged commit a401949 into RoboDurden:main Sep 20, 2023
@Jim13MC
Copy link

Jim13MC commented Sep 22, 2023

WizzardDr what is your YouTube channel? I am interested in the video you mentioned that you could make.

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

Successfully merging this pull request may close these issues.

3 participants