Skip to content

Add Mock Support for EEPROM #174

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.

Sign up for GitHub

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

Merged
merged 8 commits into from
Oct 9, 2020
Merged

Add Mock Support for EEPROM #174

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
9df75db
Don't define ostream& operator() if already defined by Apple (#171)
Oct 5, 2020
c512f28
Only include EEPROM if board supports it.
Oct 5, 2020
d82c1cf
Merge branch 'master' into tdd
Oct 5, 2020
524d352
Add EEPROM_SIZE to mega boards.
Oct 6, 2020
8ed2732
Move `EEPROM_SIZE` macro to `misc/default.yml`.
Oct 6, 2020
fadc743
Use E2END to calculate EEPROM_SIZE.
Oct 6, 2020
0ee5170
EEPROM is tested by uno and due so we don't need to add mega2560 (one…
Oct 6, 2020
4605207
Simplify reference to EEPROM library.
Oct 7, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/).

## [Unreleased]
### Added
- Support for EEPROM
- Support for mock EEPROM (but only if board supports it; added mega2560 to TestSomething config)

### Changed
- Move repository from https://github.com/ianfixes/arduino_ci to https://github.com/Arduino-CI/arduino_ci
Expand All @@ -17,6 +17,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
### Removed

### Fixed
- Don't define `ostream& operator<<(nullptr_t)` if already defined by Apple

### Security

Expand Down
1 change: 1 addition & 0 deletions SampleProjects/TestSomething/.arduino-ci.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ unittest:
platforms:
- uno
- due
- mega2560

compile:
platforms:
Expand Down
4 changes: 4 additions & 0 deletions SampleProjects/TestSomething/test/eeprom.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,13 @@
#include <Arduino.h>
#include <EEPROM.h>

#ifdef EEPROM_SIZE

unittest(length)
{
assertEqual(EEPROM_SIZE, EEPROM.length());
}

#endif

unittest_main()
7 changes: 4 additions & 3 deletions cpp/arduino/EEPROM.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,8 @@
#include <inttypes.h>
#include <avr/io.h>

// I see EEPROM_SIZE defined in various arv/io*.h files; why isn't it defined here?
#define EEPROM_SIZE (4096)
#ifdef EEPROM_SIZE

// Is this all the custom code required?
static uint8_t eeprom[EEPROM_SIZE];
inline uint8_t eeprom_read_byte( uint8_t* index ) { return eeprom[(unsigned long) index % EEPROM_SIZE]; }
Expand Down Expand Up @@ -152,4 +152,5 @@ struct EEPROMClass{
};

static EEPROMClass EEPROM;
#endif
#endif
#endif
11 changes: 11 additions & 0 deletions cpp/arduino/avr/iomxx0_1.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -351,6 +351,17 @@
Last two letters: EEAR address. */
#define __EEPROM_REG_LOCATIONS__ 1F2021

/* According to https://ww1.microchip.com/downloads/en/devicedoc/atmel-2549-8-bit-avr-microcontroller-atmega640-1280-1281-2560-2561_datasheet.pdf,
* the Atmel ATmega640/V-1280/V-1281/V-2560/V-2561/V microcontrollers each have
* 4Kbytes of EEPROM and since this file is included from things that seem
* to match that description, it would be helpful to know that EEPROM is
* available and know its size. This macro is defined for many other models,
* so we will add it here.
*
* See https://github.com/Arduino-CI/arduino_ci/issues/166#issuecomment-703904394.
*/
#define EEPROM_SIZE (4096)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It might be more appropriate to conditionally set this variable based on the board being used, which can be done in the default configuration here:
https://github.com/Arduino-CI/arduino_ci/blob/master/misc/default.yml#L102

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can certainly try that. By way of background, where did those files come from? Why does EEPROM_SIZE exist in over 40 of them?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I copied them from the Arduino project itself, arduino-1.8.5/hardware/tools/avr/avr/include/avr to get things to compile... some hand-edits were made. I have very little information on how/why Arduino structures these files.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

where did those files come from?

It's avr-libc:
https://www.nongnu.org/avr-libc/
Nothing Arduino-specific. This is just the standard open source AVR toolchain.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why does EEPROM_SIZE exist in over 40 of them?

Because there are a lot of different AVR microcontrollers.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd really love to find a way to avoid packaging all of these files in this repo but I'm at a loss for how to do so (while enabling a non-AVR compiler to supply all the #define statements some alternate way).

Copy link
Member Author

@jgfoster jgfoster Oct 6, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because there are a lot of different AVR microcontrollers.

But why isn't it defined for Mega2560? That microcontroller has EPROM in it. I was hoping that the macro would be a reliable way to determine whether the feature is present. Instead it seems that it has __EEPROM_REG_LOCATIONS__ but not the size.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The files are mostly out of the way and don't bother me much; I just find myself expecting more of them than I should given my limited knowledge!

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I notice that, while the megaAVR EEPROM library uses EEPROM_SIZE, the AVR EEPROM library uses E2END.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see now. The macros are artifacts of the implementation, not a way of giving information to clients of the library, and different implementations will have different macros. That makes sense and is reasonable.

Further investigation shows that E2END exists in 230 files and many times is (0x0). Perhaps it is a more reliable macro to use.


#define GTCCR _SFR_IO8(0x23)
#define TSM 7
#define PSRASY 1
Expand Down
4 changes: 4 additions & 0 deletions cpp/unittest/OstreamHelpers.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,4 +2,8 @@

#include <ostream>

#if (defined __apple_build_version__) && (__apple_build_version__ >= 12000000)
// defined in /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/../include/c++/v1/ostream:223:20
#else
inline std::ostream& operator << (std::ostream& out, const std::nullptr_t &np) { return out << "nullptr"; }
#endif