Skip to content
This repository has been archived by the owner on Aug 21, 2024. It is now read-only.

Introduce hardware abstraction layer #64

Open
wants to merge 8 commits into
base: main
Choose a base branch
from
Open

Conversation

orzel
Copy link

@orzel orzel commented Feb 3, 2022

Provides a better way to specify different hardware targets (+some misc fixes)

@orzel
Copy link
Author

orzel commented Sep 29, 2022

Hello ? Any problem with merging this ? It's been a while..

@zerog2k
Copy link
Owner

zerog2k commented Oct 2, 2022

hi @orzel sorry, I may have missed the PR when it was opened. I haven't done much with this project in the past year or so since my desk clocks have been mostly stable.
However, I do really like your changes. One question though, have you tested this with platformio building?
currently, it can be built with either make or with platformio - so I wonder if there are a few things that may need to be duplicated/adjusted into the platformio.ini ?

@orzel
Copy link
Author

orzel commented Oct 11, 2022

Nope, never tried platformio on this project. There's not much information on https://github.com/orzel/stc_diyclock#platformio-support
I'm willing to test and/or fix wrt to my changes, but I have this error currently, whatever the env_default i choose (note the warning):

orzel@berlioz % pio run
Warning! `env_default` configuration option in section [platformio] is deprecated and will be removed in the next release! Please use `default_envs` instead
Processing stc15w408as (platform: https://github.com/platformio/platform-intel_mcs51.git; board: stc15w408as)
---------------------------------------------------------------------------------------------------------------------------------------------------------------------------
Verbose mode can be enabled via `-v, --verbose` option
Error: Unknown board ID 'stc15w408as'

Signed-off-by: Thomas Capricelli <capricelli@sylphide-consulting.com>
@zerog2k
Copy link
Owner

zerog2k commented Oct 24, 2022

Thanks. So yes for building with platformio, you can either install and run directly from cli as you have seen (akin to traditional toolchain like make, etc), or the better way (IMHO) is running platformio in Visual Studio Code.

When I attempt to run this, I'm getting a number of errors, likely because the way platformio works, is that you can also define different target "environments", i.e. for different boards/hardward variants, etc. Because you are handling this via some defines inside the HAL stuff, I think we just need to update each of the "environments" (really target hardware families/variants), with appropriate C build flags for each, per: https://docs.platformio.org/en/stable/projectconf/section_env_build.html#examples

currently getting:

Warning! `env_default` configuration option in section [platformio] is deprecated and will be removed in the next release! Please use `default_envs` instead
Processing stc15w408as (platform: https://github.com/platformio/platform-intel_mcs51.git; board: stc15w408as)
------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
Verbose mode can be enabled via `-v, --verbose` option
CONFIGURATION: https://docs.platformio.org/page/boards/intel_mcs51/stc15w408as.html
PLATFORM: Intel MCS-51 (8051) (1.2.2) > Generic STC15W408AS
HARDWARE: STC15W408AS 11MHz, 512B RAM, 8KB Flash
PACKAGES: 
 - tool-stcgal @ 1.104.0 (1.4) 
 - toolchain-sdcc @ 1.30804.10766 (3.8.4)
LDF: Library Dependency Finder -> https://bit.ly/configure-pio-ldf
LDF Modes: Finder ~ chain, Compatibility ~ soft
Found 0 compatible libraries
Scanning dependencies...
No dependencies
Building in release mode
Compiling .pio/build/stc15w408as/src/adc.rel
Compiling .pio/build/stc15w408as/src/ds1302.rel
Compiling .pio/build/stc15w408as/src/main.rel
In file included from src/ds1302.c:10:
src/ds1302.h:7:10: error: #include expects "FILENAME" or <FILENAME>
In file included from src/main.c:13:
src/ds1302.h:7:10: error: #include expects "FILENAME" or <FILENAME>
src/main.c:23:10: error: #include expects "FILENAME" or <FILENAME>
src/main.c:192: error 20: Undefined identifier 'NUM_SW'
src/main.c:192: error 2: Initializer element is not a constant expression
src/main.c:193: error 20: Undefined identifier 'NUM_SW'
src/main.c:193: error 2: Initializer element is not a constant expression
src/main.c:231: warning 112: function 'LED_DIGITS_OFF' implicit declaration
src/main.c:236: error 20: Undefined identifier 'LED_SEGMENT_PORT'
src/main.c:240: error 20: Undefined identifier 'LED_DIGITS_PORT_BASE'
src/main.c:241: error 20: Undefined identifier 'LED_DIGITS_PORT'
src/main.c:241: error 20: Undefined identifier 'LED_DIGITS_PORT'
src/main.c:319: error 20: Undefined identifier 'SW1'
src/main.c:320: error 20: Undefined identifier 'SW2'
src/main.c:327: warning 24: index 1 is outside of the array bounds (array size is 1)
src/ds1302.c:109: error 20: Undefined identifier 'DS_CE'
src/ds1302.c:110: error 20: Undefined identifier 'DS_SCLK'
src/ds1302.c:111: error 20: Undefined identifier 'DS_CE'
src/ds1302.c:116: error 20: Undefined identifier 'DS_CE'
src/ds1302.c:124: error 20: Undefined identifier 'DS_CE'
src/ds1302.c:125: error 20: Undefined identifier 'DS_SCLK'
src/ds1302.c:126: error 20: Undefined identifier 'DS_CE'
src/ds1302.c:133: error 20: Undefined identifier 'DS_CE'
src/ds1302.c:140: error 20: Undefined identifier 'DS_CE'
src/ds1302.c:141: error 20: Undefined identifier 'DS_SCLK'
src/ds1302.c:142: error 20: Undefined identifier 'DS_CE'
src/ds1302.c:148: error 20: Undefined identifier 'DS_CE'
src/main.c:458: error 20: Undefined identifier 'ADC_LIGHT'
src/main.c:458: error 20: Undefined identifier 'ADC_TEMP'
src/main.c:459: error 20: Undefined identifier 'ADC_LIGHT'
src/main.c:459: error 20: Undefined identifier 'ADC_TEMP'
src/main.c:493: error 20: Undefined identifier 'ADC_TEMP'
src/main.c:493: error 20: Undefined identifier 'ADC_TEMP'
src/main.c:495: error 20: Undefined identifier 'ADC_LIGHT'
src/main.c:495: error 20: Undefined identifier 'ADC_LIGHT'
src/main.c:1177: error 20: Undefined identifier 'BUZZER_ON'
src/main.c:1179: error 20: Undefined identifier 'BUZZER_OFF'
src/main.c:1182: error 20: Undefined identifier 'BUZZER_OFF'
src/main.c:1189: error 20: Undefined identifier 'BUZZER_ON'
src/main.c:1191: error 20: Undefined identifier 'BUZZER_OFF'
*** [.pio/build/stc15w408as/src/ds1302.rel] Error 1
*** [.pio/build/stc15w408as/src/main.rel] Error 1
============================================================================ [FAILED] Took 0.84 seconds ============================================================================

Environment    Status    Duration
-------------  --------  ------------
stc15w408as    FAILED    00:00:00.845
====================================================================== 1 failed, 0 succeeded in 00:00:00.845 ======================================================================

@zerog2k
Copy link
Owner

zerog2k commented Oct 24, 2022

Clarifying a bit further, platformio is independent from make, meaning it doesnt see or use anything in the Makefile. So if we have some mandatory new define, it would need to be set somehow (likely via build_flags) in the platformio.ini.
So for example, seems like we need to decide on what HARDWARE value should be for each of the board types.

NB: inside the intel_mcs51 platform there are board definitions which already take care of settings some of the particular things you'd previously had to manage explicitly setting in the makefile when passing options to sdcc/etc, e.g. what we currently pass as STCCODESIZE or SDCCOPTS in makefile come to mind.

example: https://github.com/platformio/platform-intel_mcs51/blob/develop/boards/STC15W408AS.json

But application logic like HARDWARE define is something we need to pass explicitly (i.e. build_flags) via platformio.ini, unless there is already some reasonable default inside the code/macros itself.

@zerog2k
Copy link
Owner

zerog2k commented Oct 24, 2022

just a quick test, I was able to add this to [env:stc15w408as] section of platformio.ini:

build_flags = 
    -DHARDWARE='"hal/default.h"'

and it did build (with some other warnings), but I have not checked functionality. (Note the single quote wrapping doublequotes is important here.)

 *  Executing task: platformio run 

Warning! `env_default` configuration option in section [platformio] is deprecated and will be removed in the next release! Please use `default_envs` instead
Processing stc15w408as (platform: https://github.com/platformio/platform-intel_mcs51.git; board: stc15w408as)
------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
Verbose mode can be enabled via `-v, --verbose` option
CONFIGURATION: https://docs.platformio.org/page/boards/intel_mcs51/stc15w408as.html
PLATFORM: Intel MCS-51 (8051) (1.2.2) > Generic STC15W408AS
HARDWARE: STC15W408AS 11MHz, 512B RAM, 8KB Flash
PACKAGES: 
 - tool-stcgal @ 1.104.0 (1.4) 
 - toolchain-sdcc @ 1.30804.10766 (3.8.4)
LDF: Library Dependency Finder -> https://bit.ly/configure-pio-ldf
LDF Modes: Finder ~ chain, Compatibility ~ soft
Found 0 compatible libraries
Scanning dependencies...
No dependencies
Building in release mode
Compiling .pio/build/stc15w408as/src/adc.rel
Compiling .pio/build/stc15w408as/src/ds1302.rel
Compiling .pio/build/stc15w408as/src/main.rel
src/main.c:448: warning 59: function 'add_BCD' must return value
Linking .pio/build/stc15w408as/firmware.hex
?ASlink-Warning-Definition of public symbol '__mcs51_genRAMCLEAR' found more than once:
   Library: '/Users/jens/.platformio/packages/toolchain-sdcc/bin/../share/sdcc/lib/small/mcs51.lib', Module: 'crtclear.rel'
   Library: '/usr/local/share/sdcc/lib/small/mcs51.lib', Module: 'crtclear.rel'
?ASlink-Warning-Definition of public symbol '__mcs51_genXINIT' found more than once:
   Library: '/Users/jens/.platformio/packages/toolchain-sdcc/bin/../share/sdcc/lib/small/mcs51.lib', Module: 'crtxinit.rel'
   Library: '/usr/local/share/sdcc/lib/small/mcs51.lib', Module: 'crtxinit.rel'
?ASlink-Warning-Definition of public symbol '__modsint_PARM_2' found more than once:
   Library: '/Users/jens/.platformio/packages/toolchain-sdcc/bin/../share/sdcc/lib/small/libint.lib', Module: '_moduint.rel'
   Library: '/usr/local/share/sdcc/lib/small/libint.lib', Module: '_moduint.rel'
?ASlink-Warning-Definition of public symbol '__mcs51_genXRAMCLEAR' found more than once:
   Library: '/Users/jens/.platformio/packages/toolchain-sdcc/bin/../share/sdcc/lib/small/mcs51.lib', Module: 'crtxclear.rel'
   Library: '/usr/local/share/sdcc/lib/small/mcs51.lib', Module: 'crtxclear.rel'
?ASlink-Warning-Definition of public symbol '__divsint_PARM_2' found more than once:
   Library: '/Users/jens/.platformio/packages/toolchain-sdcc/bin/../share/sdcc/lib/small/libint.lib', Module: '_divuint.rel'
   Library: '/usr/local/share/sdcc/lib/small/libint.lib', Module: '_divuint.rel'
?ASlink-Warning-Definition of public symbol '__XPAGE' found more than once:
   Library: '/Users/jens/.platformio/packages/toolchain-sdcc/bin/../share/sdcc/lib/small/mcs51.lib', Module: 'crtpagesfr.rel'
   Library: '/usr/local/share/sdcc/lib/small/mcs51.lib', Module: 'crtpagesfr.rel'
?ASlink-Warning-Definition of public symbol '__sdcc_gsinit_startup' found more than once:
   Library: '/Users/jens/.platformio/packages/toolchain-sdcc/bin/../share/sdcc/lib/small/mcs51.lib', Module: 'crtstart.rel'
   Library: '/usr/local/share/sdcc/lib/small/mcs51.lib', Module: 'crtstart.rel'
?ASlink-Warning-Definition of public symbol '__modsint' found more than once:
   Library: '/Users/jens/.platformio/packages/toolchain-sdcc/bin/../share/sdcc/lib/small/libint.lib', Module: '_modsint.rel'
   Library: '/usr/local/share/sdcc/lib/small/libint.lib', Module: '_modsint.rel'
?ASlink-Warning-Definition of public symbol '__divsint' found more than once:
   Library: '/Users/jens/.platformio/packages/toolchain-sdcc/bin/../share/sdcc/lib/small/libint.lib', Module: '_divsint.rel'
   Library: '/usr/local/share/sdcc/lib/small/libint.lib', Module: '_divsint.rel'
?ASlink-Warning-Definition of public symbol '__sdcc_external_startup' found more than once:
   Library: '/Users/jens/.platformio/packages/toolchain-sdcc/bin/../share/sdcc/lib/small/libsdcc.lib', Module: '_startup.rel'
   Library: '/usr/local/share/sdcc/lib/small/libsdcc.lib', Module: '_startup.rel'
Checking size .pio/build/stc15w408as/firmware.hex
Advanced Memory Usage is available via "PlatformIO Home > Project Inspect"
Flash: [=====     ]  46.8% (used 3833 bytes from 8192 bytes)
============================================================================== [SUCCESS] Took 5.34 seconds ==============================================================================

Environment    Status    Duration
-------------  --------  ------------
stc15w408as    SUCCESS   00:00:05.344
============================================================================== 1 succeeded in 00:00:05.344 ==============================================================================
 *  Terminal will be reused by tasks, press any key to close it. 

@orzel
Copy link
Author

orzel commented Oct 29, 2022

Ok. I see. platformio.ini doesn't seem to be specific to vscode, is it ? Rather to platformio, no ?
So, I guess it's a matter of

  • removing the current three env, which are 'mcu' specific, rather than "clock hardware/version/varian"
  • defining 3 new env related to actual clock hardware, with proper names, and comments (can be taken from hal/*.h)
  • adding the proper -DHARDWARE='"hal/xxx.h"' to each
    ?
    I currently use the three names "default", "modelc", "stc15w408as", but i would be happy if you come with better ones. stc15w408as is the one I have

@zerog2k
Copy link
Owner

zerog2k commented Dec 10, 2022

platformio.ini doesn't seem to be specific to vscode

correct, platformio is the build system, think of it as replacing make. It has very good integration with VSCode, but can be run standalone from cli.

Yes unfortunately, the different mcus (usually one of stc15f204ea, stc15w404as, stc15w408as) have been seen mixed and matched with different actual hardware variants (differing pinouts/pcb, and in some cases different peripherals).
So there is some "muddying of the water" between actual pcb layouts and mcu. If we started from scratch, I'd probably name them all like variant A, B, C - but unfortunately they have been mixed with the kind of mcu they shipped with. So it's not exactly perfect. Probably should be mix of mcu + pcb, but I only have one variant to test with.
So not sure what the best solution is here, but open to suggestion.

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

Successfully merging this pull request may close these issues.

2 participants