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

Task 16 backend #52

Open
wants to merge 6 commits into
base: devel
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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: 3 additions & 0 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,9 @@ target_sources (Ardrivo PRIVATE

include/Ardrivo/WiFi.h
src/Ardrivo/WiFi.cpp

include/Ardrivo/FrameBufferAccess.h
src/Ardrivo/FrameBufferAccess.cpp
)
if (SMCE_ARDRIVO_MQTT)
target_sources (Ardrivo PRIVATE include/Ardrivo/MQTT.h src/Ardrivo/MQTT.cpp)
Expand Down
37 changes: 37 additions & 0 deletions include/Ardrivo/FrameBufferAccess.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
//
// Created by danie on 2021-11-02.
//
Comment on lines +1 to +3
Copy link
Contributor

Choose a reason for hiding this comment

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

Missing copyright


#ifndef FRAMEBUFFER_ACCESS_H
#define FRAMEBUFFER_ACCESS_H

#include "SMCE_dll.hpp"

enum SMCE_Pixel_Format
{
RGB888, // RRRRRRRRGGGGGGGGBBBBBBBB // SMCE extension
RGB444, // GGGGBBBB----RRRR
};

class SMCE__DLL_RT_API FramebufferAccess {
private:
std::size_t m_key = 0;
SMCE_Pixel_Format m_format = RGB888;
bool m_begun = false;

public:
int begin(std::uint16_t width, std::uint16_t height, SMCE_Pixel_Format format, std::uint8_t fps);
void end();
bool read(void* buffer);
bool write(void* buffer);

[[nodiscard]] int width() const;
[[nodiscard]] int height() const;
[[nodiscard]] int bitsPerPixel() const;
[[nodiscard]] int bytesPerPixel() const;

void horizontalFlip(bool flipped);
void verticalFlip(bool flipped);
};
Comment on lines +10 to +35
Copy link
Member

Choose a reason for hiding this comment

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

This is a terrible API
I can see that you copied it from the OV767X header, which is also a terrible API which I cannot alter for compatibility with third-party software.

Copy link
Member

Choose a reason for hiding this comment

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

Keep in mind that this is an internal header, meant for library patches to use. It is NOT for Arduino user code, and thus NEEDS to provide an equivalent of the FrameBuffer interface, not the shitty OV767X one


#endif // FRAMEBUFFER_ACCESS_H
41 changes: 41 additions & 0 deletions include/Ardrivo/WString.h
Original file line number Diff line number Diff line change
Expand Up @@ -181,6 +181,47 @@ class SMCE__DLL_RT_API String {
[[nodiscard]] bool operator>(const String& s) const noexcept;
[[nodiscard]] bool operator>=(const String& s) const noexcept;

String& operator+=(const String& rhs) {
concat(rhs);
return (*this);
Comment on lines +184 to +186
Copy link
Contributor

Choose a reason for hiding this comment

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

Can't this be templated much like the concat function?

Copy link
Member

Choose a reason for hiding this comment

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

I second that

Copy link
Member

Choose a reason for hiding this comment

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

And why do all these return statements have their expression parenthesized?

}
String& operator+=(const char* cstr) {
concat(cstr);
return (*this);
}
String& operator+=(char c) {
concat(c);
return (*this);
}
String& operator+=(unsigned char num) {
concat(num);
return (*this);
}
String& operator+=(int num) {
concat(num);
return (*this);
}
String& operator+=(unsigned int num) {
concat(num);
return (*this);
}
String& operator+=(long num) {
concat(num);
return (*this);
}
String& operator+=(unsigned long num) {
concat(num);
return (*this);
}
String& operator+=(float num) {
concat(num);
return (*this);
}
String& operator+=(double num) {
concat(num);
return (*this);
}

friend SMCE__DLL_RT_API String operator+(const String&, const char*);
friend SMCE__DLL_RT_API String operator+(const char*, const String&);
};
Expand Down
139 changes: 139 additions & 0 deletions src/Ardrivo/FrameBufferAccess.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,139 @@
//
// Created by danie on 2021-11-02.
//
Comment on lines +1 to +3
Copy link
Contributor

Choose a reason for hiding this comment

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

missing copyright


#include <array>
#include <climits>
#include <iostream>
#include <SMCE/BoardView.hpp>
#include "../../include/Ardrivo/FrameBufferAccess.h"
Copy link
Contributor

Choose a reason for hiding this comment

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

Shoudn't use relative paths


namespace smce {
extern BoardView board_view;
extern void maybe_init();
} // namespace smce

int FramebufferAccess::begin(std::uint16_t width, std::uint16_t height, SMCE_Pixel_Format format, std::uint8_t fps) {
Copy link
Contributor

Choose a reason for hiding this comment

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

shoudn't this also take they fb key? (even if its just 0 for now)

const auto error = [=](const char* msg) {
std::cerr << "ERROR: FramebufferAccess::begin(" << width << "x" << height << ", " << format << ", " << fps
<< "): " << msg << std::endl;
return -1;
};
if (m_begun) {
std::cerr << "FramebufferAccess::begin: device already active" << std::endl;
return -1;
}
switch (format) {
case RGB888:
case RGB444:
m_format = format;
break;
default:
return error("Unknown format");
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe be a bit more descriptive so that we know where this comes from

}

auto fb = smce::board_view.frame_buffers[m_key];
if (!fb.exists())
return error("Framebuffer does not exist");
if (fb.direction() != smce::FrameBuffer::Direction::in)
return error("Framebuffer not in input mode");

fb.set_width(width);
fb.set_height(height);
fb.set_freq(fps);

m_begun = true;
return 0;
}

void FramebufferAccess::end() {
if (!m_begun) {
std::cerr << "FramebufferAccess::end: device inactive" << std::endl;
return;
}
auto fb = smce::board_view.frame_buffers[m_key];
fb.set_width(0);
fb.set_height(0);
fb.set_freq(0);
m_begun = false;
}

bool FramebufferAccess::read(void* buffer) {
Copy link
Contributor

Choose a reason for hiding this comment

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

uint8_t* instead of void*

if (!m_begun) {
std::cerr << "FramebufferAccess::read: device inactive" << std::endl;
return false;
}
using ReadType = std::add_const_t<decltype(&smce::FrameBuffer::read_rgb888)>;
constexpr ReadType format_read[2] = {
&smce::FrameBuffer::read_rgb888,
&smce::FrameBuffer::read_rgb444,
};
(smce::board_view.frame_buffers[m_key].*format_read[m_format])(
{static_cast<std::byte*>(buffer), static_cast<std::size_t>(bitsPerPixel() * width() * height() / CHAR_BIT)});
return true;
}

bool FramebufferAccess::write(void* buffer) {
Copy link
Contributor

Choose a reason for hiding this comment

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

const uint8_t*

if (!m_begun) {
std::cerr << "FramebufferAccess::write: device inactive" << std::endl;
return false;
}
using WriteType = std::add_const_t<decltype(&smce::FrameBuffer::write_rgb888)>;
constexpr WriteType format_write[2] = {
&smce::FrameBuffer::write_rgb888,
&smce::FrameBuffer::write_rgb444,
};
(smce::board_view.frame_buffers[m_key].*format_write[m_format])(
{static_cast<std::byte*>(buffer), static_cast<std::size_t>(bitsPerPixel() * width() * height() / CHAR_BIT)});
return true;
}

int FramebufferAccess::width() const {
if (!m_begun) {
std::cerr << "FramebufferAccess::width: device inactive" << std::endl;
return 0;
}
return smce::board_view.frame_buffers[m_key].get_width();
}

int FramebufferAccess::height() const {
if (!m_begun) {
std::cerr << "FramebufferAccess::height: device inactive" << std::endl;
return 0;
}
return smce::board_view.frame_buffers[m_key].get_height();
}

constexpr std::array<std::pair<int, int>, 2> bits_bytes_pixel_formats{{{24, 3}, {16, 2}}};

int FramebufferAccess::bitsPerPixel() const {
if (!m_begun) {
std::cerr << "FramebufferAccess::bitsPerPixel: device inactive" << std::endl;
return 0;
}
return bits_bytes_pixel_formats[m_format].first;
Comment on lines +109 to +114
Copy link
Contributor

Choose a reason for hiding this comment

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

Shoudn't this be something on BoardView::Framebuffer itself? seems awfully useful thing to have and weird for it to be present in Ardrivo only

}

int FramebufferAccess::bytesPerPixel() const {
if (!m_begun) {
std::cerr << "FramebufferAccess::bytesPerPixel: device inactive" << std::endl;
return 0;
}
return bits_bytes_pixel_formats[m_format].second;
}

void FramebufferAccess::horizontalFlip(bool flipped) {
if (!m_begun) {
std::cerr << "OV767X::horizontalFlip: device inactive" << std::endl;
return;
}
smce::board_view.frame_buffers[m_key].needs_horizontal_flip(flipped);
}

void FramebufferAccess::verticalFlip(bool flipped) {
if (!m_begun) {
std::cerr << "OV767X::verticalFlip: device inactive" << std::endl;
return;
}
smce::board_view.frame_buffers[m_key].needs_vertical_flip(flipped);
}