MarlinFirmware / Marlin

Marlin is an optimized firmware for RepRap 3D printers based on the Arduino platform. Many commercial 3D printers come with Marlin installed. Check with your vendor if you need source code for your specific machine.
https://marlinfw.org
GNU General Public License v3.0
16.23k stars 19.22k forks source link

HAL support definition and BSP #2139

Closed fmalpartida closed 8 years ago

fmalpartida commented 9 years ago

Little thread to start laying down the tracks for a HAL and BSP such that the base core code is abstracted out from the underlaying HW/chip.

Current suggestion is to split in two: HAL - Hardware abstraction to enable access to chipsets devices BSP - Board support package or chip support package, chip specifics to enable running the base code.

Boundary conditions?

daid commented 9 years ago

Adding other support then AVR in the same work should not be done. (Maybe other 8 bitters) Due to the simple fact that keeping AVR support in an ARM port will cause a very messy codebase as well as cause problems because AVR is very limited while ARM has much more speed and storage.

It also makes it even more impossible to have a stable code base and to test changes.

Also, what has "binary space partitioning" to do with it?

nophead commented 9 years ago

BSP = board support package in this context.

JustAnother1 commented 9 years ago

I would assume that the HAL is only an API to what the BSP implements. But if you list it as two separate things could you explain what stuff is part of BSP and what is part of HAL?

I agree with @daid the difference between 8bit and 32bit is too big to have a healthy common code base.

fmalpartida commented 9 years ago

JustAnother1 is very much on the ball here, the HAL in nothing more than a common API/interface for accessing the HW and the BSP is its implementation.

In portions we already have it in place with the pin definitions and mappings but it is not extended to other HW devices where the entire code base is relying on the Arduino API.

I do agree that writing code for an 8 bit controller and a 32 bit controller opens up a good can of worms, but we have to acknowledge that the core of the code should be common.

JustAnother1 commented 9 years ago

Defining the Arduino API as being (part of) the HAL could be a solution for those parts that rely on the Arduino API.

This would mean that BSPs for other Chips would then need to implement the Arduino API. I don't know if that makes sense. It might be better to reorganize the code to reduce the complexity of the HAL.

I would suggest as a first step to restructure the code and have the current API used as HAL. This way one can see what still needs improvement. The next step would then be to create a second BSP (first BSP is AVR). This again is a situation where issues might arise that show potential for optimization. Doing this first step with the actual code helps this discussion to not drift into theoretical spheres too much.

A important point will be the build Environment and the handling of the configuration (which features shall be enabled / disabled in the build). I assume that the Arduino IDE will not be able to handle this. So what will the build system look like? Makefile?

@bkubicek already had the Idea of having more modularity in Firmwares. He might be able to give some insight here.

fmalpartida commented 9 years ago

@JustAnother1 a agree here. In a previous thread where I shared this thought, I was mentioning that the Arduino API is indeed a HAL.

The line of thought, was to "harmonise" the current use of the current code to go down the root of using the current Arduino API. In fact, most of the code is written for Arduino. There are gaps such as those associated to interrupts, timers, ... But the base is there. In my mind, the first step would be to identify such gaps and refactor the code a bit to have a clean HAL cut. Currently fastIO is an optimisation of the Arduino digital interface (too slow) however it maintains the same API and hence a lot of libraries out there can be used.

Builds... This is a very good question that I was trying to get my head around. Perhaps the simplest form would be makefiles, but we have to recognise that I lot of people may struggle with it.

Lets see what others think about this first layer and what insights @bkubicek can share.

Wackerbarth commented 9 years ago

I think that you are looking too low when you consider the Arduino API as your HAL. Instead,you should be looking at functional units (Positioners, Heaters, Sensors, etc.) Now one implementation could utilize the Arduino API to implement those functions. But there could also be other implementations. And, for an initial cut, you even could implement it on 32-bit hardware.

However, I do agree that the 32-bit systems will quickly evolve into complexity which cannot be ported to the existing 8 bit hardware. Therefore, it is reasonable for that development to take place in another repository.

thinkyhead commented 9 years ago

Some top-down or "middle-down" refactoring of Marlin entities into classes is definitely in the mid-term plan. We should have classes for all the main abstractions in Marlin, as you say Positioners, Heaters, Sensors, and combinations of those such as Extruders (>=1 Heaters, >=1 Positioners, >=1 Sensors, <=1 Tool-changers). We don't need to instantiate these objects at run-time (although that would be fine, if it was all done once in the Setup). They'll just be singletons and arrays, but we will be able to enjoy some extra benefits of C++ syntax, encapsulation, etc. Presumably the code will be easier to read. Sometimes this process makes code shrink. :children_crossing:

Wackerbarth commented 9 years ago

You should take a look at https://github.com/mikaelpatel/Cosa.git for some inspiration for the structure of an alternative to the style of the Arduino Wiring API. He breaks things down into libraries. IMHO it makes things more readable because there is not a monolithic piece of code, much of which is omitted by failing #ifdef's

thinkyhead commented 9 years ago

@Wackerbarth That looks really interesting. How big is a build of "Blink" with it, I wonder? (Ok, ok, not a good benchmark, because larger programs may end up smaller.)

It is true, we can use const bool, const int, etc., and just allow the optimizer to skip unreachable code blocks. If nothing else, the code will look cleaner. It's not possible to make a const class or struct in C++ with the assignments built into the declaration, but namespace works:

namespace config {
  const int SERIAL_PORT = 0;
  const int EXTRUDERS = 1;
  ...
};

But… will the compiler strip unused variables, or just warn about them? Because const vars are no good for conditional-compilation of declarations.

nophead commented 9 years ago

The big difference with unreachable code versus code omitted with ifdef is that the former still needs to compile, so all the definitions it uses must be in place. I.e. definitions for hardware you haven't got.

Wackerbarth commented 9 years ago

Yes, but all of those definitions can be supplied by default in the base class and overridden when actually used.

daid commented 9 years ago

Anything that has runtime overhead needs to be avoided on AVR. (Or is unacceptable in case of the stepper routines. Hench the fastio.h which has zero overhead. On 23 May 2015 12:01, "Richard Wackerbarth" notifications@github.com wrote:

Yes, but all of those definitions can be supplied by default in the base class and overridden when actually used.

— Reply to this email directly or view it on GitHub https://github.com/MarlinFirmware/Marlin/issues/2139#issuecomment-104877731 .

Wackerbarth commented 9 years ago

I was referring to compile time overrides. The compilers understand how to process constants.

daid commented 9 years ago

Don't make assumptions on the avr-gcc, it can do really stupid things sometimes...

On Sat, May 23, 2015 at 12:48 PM, Richard Wackerbarth < notifications@github.com> wrote:

I was referring to compile time overrides. The compilers understand how to process constants.

— Reply to this email directly or view it on GitHub https://github.com/MarlinFirmware/Marlin/issues/2139#issuecomment-104881753 .

thinkyhead commented 9 years ago

Definitely don't make assumptions. It's never bad to be more scientific about build size and performance. Here are some sample build sizes with Arduino 1.6.1 that are just an interesting side-note…

 March 27:
 51,874 : Default configuration
 95,562 : G3D_PANEL
 95,568 : REPRAP_DISCOUNT_SMART_CONTROLLER
106,960 : REPRAP_DISCOUNT_FULL_GRAPHIC_SMART_CONTROLLER
+11,398 : REPRAP_DISCOUNT_FULL_GRAPHIC_SMART_CONTROLLER vs G3D_PANEL

 May 24:
 51,624 : Default configuration (-250)
 50,600 : Without thermal protection
 95,852 : G3D_PANEL (+290)
 95,858 : REPRAP_DISCOUNT_SMART_CONTROLLER (+290)
107,154 : REPRAP_DISCOUNT_FULL_GRAPHIC_SMART_CONTROLLER (+194)
+11,296 : REPRAP_DISCOUNT_FULL_GRAPHIC_SMART_CONTROLLER vs G3D_PANEL (-102)

I encourage that we do testing and comparison at each stage to make sure code is compiling optimally with the latest toolkit. Code such as the following is easily caught by the optimizer, but there may be some interesting counterexamples. It will be good to know what we should avoid.

const int some_value = 100;
/* begin non-compiled code */
if (some_value < 100) {
  // unreachable code
}
/* end non-compiled code */
jbrazio commented 8 years ago

Thank you for your interest making Marlin better and reporting this issue but this topic has been open for a long period of time without any further development. Marlin has been under heavy development for the past couple of months and moving to it's last mile to finish the RC cycle and release Marlin v1.1.0. We suggest you to try out the latest RCBugfix branch and reopening this issue if required.

github-actions[bot] commented 2 years ago

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.