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

Adding bitmap_printer_device and session_time device and converting epson_lx810 to use it. #8863

Merged
merged 7 commits into from
Jan 7, 2022

Conversation

goldnchild
Copy link
Contributor

Hi guys,

I wanted you to have a look at this to see if I'm on the right track.

Some background on my bitmap_printer_device:

I noticed that all of the dot matrix printers have very similar needs, a cr_stepper, pf_stepper and a page bitmap to draw on, so why not have a "bitmap_printer_device" that will take care of that so you don't have to duplicate a lot of code. I was thinking that "bitmap_printer_device" could be renamed "dot_printer_device" possibly.

It watches when you move the pf stepper and if you pass the bottom margin, saves out a page as a png and moves to a fresh page.

The steppers use the "standard" pattern from the steppers.c file, just doing a bitswap to make the patterns fit that standard sequence. Reversing the bits has the effect of reversing the direction of movement.

The printhead is a little bit more complicated, so it's left to the driver to do that.

The purpose of the session_time device is just to give a unique filename based on the current time the session started, with the ability to reset the session_time and page number if you were going to start a printout and wanted it to start at page 1.

So for example, it will save filenames like the following in the "imagewriter" directory:

imagewriter_2021-09-17_22-12-12_ct486-board2-comat-serport1_page_006.png
imagewriter_2021-09-18_00-17-21_a500-rs232_page_001.png
imagewriter_2021-09-18_19-07-59_coco2-rs232_page_001.png
imagewriter_2021-09-20_06-00-15_c64-user-rs232-rs232_page_001.png
imagewriter_2021-09-30_20-34-11_bbcb-rs423_page_001.png
imagewriter_2021-10-05_04-24-25_apple2p-sl1-ssc-rs232_page_001.png
imagewriter_2021-11-21_09-18-39_mac512k-rs232a_page_001.png

And since the session time and the tag is unique, you can have multiple printers on a system and they will save as different filenames.

for the silentype printer:
silentype_printer_2021-11-19_04-51-25_apple2p-sl1-silentype_page_001.png

for the ap2000 printer:
ap2000_2021-11-21_08-50-32_apple2p-sl1-uniprint-prn_page_001.png

I've converted the lx810l to use the bitmap_printer_device, which wasn't too difficult. Since the screen/page display and the steppers are part of the bitmap_printer_device, there's less code in the epson_lx810l.cpp/h files.

Future enhancements could be to track the speed of the steppers to have more precise position than just whole steps.

I was also interested in making it so that the bitmap would be larger than just a single page (like 2 pages) to accommodate printing on the page boundary. But for now, we'll just have a single page to work with.

#define PAPER_WIDTH 1024
#define PAPER_HEIGHT 576

#define PAPER_WIDTH 1024 // 120 dpi * 8.5333 inches
Copy link
Contributor

Choose a reason for hiding this comment

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

For constants, we like to use the C++ "static constexpr int PAPER_WIDTH = 1024;" form now, as you may have seen me converting the A2 drivers to. This gives a little more compile-time safety checking.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ok (going to read up on constexpr)

@@ -409,6 +398,9 @@ uint8_t epson_lx810l_device::porta_r(offs_t offset)

LOG("%s: lx810l_PA_r(%02x): result %02x\n", machine().describe_context(), offset, result);

// Update the printhead display, only put this here since this routine gets called frequently
m_bitmap_printer->set_printhead_color( m_e05a30->ready_led() ? 0x55ff55 : 0x337733, 0x0);
Copy link
Contributor

Choose a reason for hiding this comment

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

This should really be tied to whatever sets the ready LED directly, and also the colors should be defined by the bitmap_printer device itself rather than the callers. Something like m_bitmap_printer->set_printhead_ready(true);

Copy link
Contributor Author

Choose a reason for hiding this comment

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

One way to do this would be to make a callback from the bitmap_printer_device::screen_update_bitmap routine that would update the leds.

uint32_t bitmap_printer_device::screen_update_bitmap(screen_device &screen,
bitmap_rgb32 &bitmap, const rectangle &cliprect)

Another would be to set the state directly from the e05a30 callbacks in epson_lx810.h,

int ready_led() { return !m_centronics_busy; }


DECLARE_WRITE_LINE_MEMBER(e05a30_centronics_ack) { output_ack(state); }
DECLARE_WRITE_LINE_MEMBER(e05a30_centronics_busy) { output_busy(state);
                        m_bitmap_printer->set_printhead_ready(!state); }
DECLARE_WRITE_LINE_MEMBER(e05a30_centronics_perror) { output_perror(state); }
DECLARE_WRITE_LINE_MEMBER(e05a30_centronics_fault) { output_fault(state); }
DECLARE_WRITE_LINE_MEMBER(e05a30_centronics_select) { output_select(state); }

I was thinking that the center printhead block would be the green ready LED and the border of the printhead block would be a red "error" LED (like paper error).

Allowing the caller to set the colors allows a driver like the silentype to use a different color scheme (as it doesn't have any leds). I liked a two tone beige printhead color for the silentype which kinda matched the silentype's exterior colors (though the actual printhead is white plastic). Perhaps the caller could set the printhead ready and error colors and on would be the color specified, off be 1/2 or 1/4 of the intensity of the color.

uint8_t m_fakemem;
bitmap_rgb32 m_bitmap;
int m_in_between_offset = 0; // in between cr_stepper phases
Copy link
Contributor

Choose a reason for hiding this comment

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

If these default values are important, they should be set in the device_start/device_reset members (depending on if reset actually changes them).


copyscrollbitmap(bitmap, m_page_bitmap, 0, nullptr, 1, &scrolly, cliprect);

bitmap.plot_box(0, bitmap.height() - m_distfrombottom - m_ypos, m_paperwidth, 2, 0xEEE8AA); // draw a line on the very top of the top edge of page
Copy link
Contributor

Choose a reason for hiding this comment

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

These colors should be constants rather than bare magic numbers.

@@ -0,0 +1,125 @@
// license:BSD-3-Clause
// copyright-holders:
Copy link
Contributor

Choose a reason for hiding this comment

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

Don't forget your copyright assignment :)

@@ -0,0 +1,182 @@
// license:BSD-3-Clause
Copy link
Contributor

@rb6502 rb6502 Nov 24, 2021

Choose a reason for hiding this comment

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

I understand the need for something like this and I like the feature set, but I'm not sure this is exactly the right way to do it. I need architecture advice from @cuavas on this. It feels like something that should be in the core rather than a device, and it shouldn't assume that its users are printers (I could see it potentially being used for plotters, paper tape and card punches, and stuff of that nature).

I'm also not a fan of the name "session time device", because it has very little to do with timing.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I see there's already similar functionality to convert tags to filenames (looking at my nvram directories) and I wanted a timestamp that would allow filename sorting to keep things in order. So when I launch the file viewer from Ubuntu's file manager (which I think is eog ) when I hit the right arrow I get the next image in the sequence. If I type "eog snap/ap2000" from the command line, and hit the left arrow I get the most recent image printed.

nvram/apple2e:
sl1_grappler_prn_ap2000_eeprom
sl1_grappler_prn_etibuffer_prn_ap2000_eeprom
sl1_grappler_prn_etibuffer_prn_etibuffer_prn_ap2000_eeprom
sl1_grapplus_prn_ap2000_eeprom

I also notice that there's a timestamp functionality in video.cpp's open_next

//-------------------------------------------------
// open_next - open the next non-existing file of
// type filetype according to our numbering
// scheme
//-------------------------------------------------

std::error_condition video_manager::open_next(emu_file &file, const char *extension, uint32_t added_index)
{
...
// handle %t in the template (for timestamp)
std::string snaptime("%t");
int pos_time = snapstr.find(snaptime);

    if (pos_time != -1)
    {
            char t_str[15];
            const std::time_t cur_time = std::time(nullptr);
            strftime(t_str, sizeof(t_str), "%Y%m%d_%H%M%S", std::localtime(&cur_time));
            strreplace(snapstr, "%t", t_str);
    }

Copy link
Contributor Author

@goldnchild goldnchild Nov 27, 2021

Choose a reason for hiding this comment

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

Maybe bitmap_printer_filename.cpp?

Also it doesn't really need to be a separate file/device, it could be integrated directly into bitmap_printer, the rationale being that the code would be easier to understand if it were separate.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

My string functions to change colons to underscores aren't really pretty, so I should replace them with something from

std::string running_machine::nvram_filename(device_t &device) const

           std::string tag(device.tag());
            tag.erase(0, 1);
            strreplacechr(tag,':', '_');

Copy link
Contributor Author

@goldnchild goldnchild Dec 17, 2021

Choose a reason for hiding this comment

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

I was thinking about making a copy of video_manager::open_next and adding some new % specifiers like %f = full tag %b = basename %s=session time %p = page number along with a default snapname.

In the meantime, I made a version of write_snapshot_to_file that would just call video_manager::open_next and save it in the driver's directory like it was a regular F12 snapshot.

void bitmap_printer_device::write_snapshot_to_file(std::string directory, std::string name)
{
	emu_file file(machine().options().snapshot_directory(), OPEN_FLAG_WRITE | OPEN_FLAG_CREATE | OPEN_FLAG_CREATE_PATHS);
	std::error_condition const filerr = machine().video().open_next(file, "png");

	if (!filerr)
	{
		static const rgb_t png_palette[] = { rgb_t::white(), rgb_t::black() };

		// save the paper into a png
		util::png_write_bitmap(file, nullptr, m_page_bitmap, 2, png_palette);
	}
}

@@ -399,7 +405,8 @@ uint8_t epson_lx810l_device::porta_r(offs_t offset)
LOG("%s: lx810l_PA_r(%02x): result %02x\n", machine().describe_context(), offset, result);

// Update the printhead display, only put this here since this routine gets called frequently
m_bitmap_printer->set_printhead_color( m_e05a30->ready_led() ? 0x55ff55 : 0x337733, 0x0);
// m_bitmap_printer->set_printhead_color( m_e05a30->ready_led() ? 0x55ff55 : 0x337733, 0x0);
m_bitmap_printer->set_led_state(0, !ioport("PAPEREND")->read());
Copy link
Contributor

Choose a reason for hiding this comment

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

This should be changed to use a required_ioport device so we aren't doing string-based device lookup at runtime. I use those all over the A2 drivers, look for m_kbspecial in apple2e.cpp for instance.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ok, I changed every one I could find


public:
void set_led_state(int led, int value) { m_led_state[led] = value; m_num_leds = std::max(m_num_leds, led); }
Copy link
Contributor

Choose a reason for hiding this comment

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

If the LEDs have consistent meanings, define an public enum inside the class to give them meaningful names like bitmap_printer_device::LED_READY and stuff like that.

@goldnchild
Copy link
Contributor Author

goldnchild commented Dec 23, 2021

I had an idea to do the filenames, it would be processing a snap string before sending it to video_manager::open_next so that it handles %b (basetag), %f (fulltag), %s (session time)

so before calling open_next, we get a printersnap string (possibly something like machine().options().print_snap_name()) and do the template replacements:

// std::error_condition const filerr = machine().video().open_next(file, "png");
std::error_condition const filerr = open_next_printer(file, "png");

// a routine to process the snap string before passing it on:


void bitmap_printer_device::bitmap_printer_process(std::string& snapstr)
{
// handle %s session timestamp
	std::string snaptime("%s");
	int pos_time = snapstr.find(snaptime);

	if (pos_time != -1)
	{
		char t_str[15];
		strftime(t_str, sizeof(t_str), "%Y%m%d_%H%M%S", std::localtime(&session_time));
		strreplace(snapstr, "%s", t_str);
	}

// handle %f  full tag
    std::string s_tag(owner()->tag());
    s_tag.erase(0, 1);
    strreplacechr(s_tag,':', '_');
	strreplace(snapstr, "%f", s_tag);

// handle %b basetag
	strreplace(snapstr, "%b", owner()->basetag());
}

std::error_condition bitmap_printer_device::open_next_printer(emu_file &file, const char *extension)
{
	const char *snapname = "%g_%b/%s_%g_%f_%i";  // or machine().options().printsnap_name();
	std::string snapstr(snapname);
	bitmap_printer_process(snapstr);		
	return open_next(file, extension, snapstr.c_str(), 0);
}

//-------------------------------------------------
//  open_next - open the next non-existing file of
//  type filetype according to our numbering
//  scheme
//-------------------------------------------------

std::error_condition bitmap_printer_device::open_next(emu_file &file, const char *extension, const char * snapname, uint32_t added_index)
{
	u32 origflags = file.openflags();
	// handle defaults
//	const char *snapname = machine().options().snap_name();  // passing this in as a parameter

// so a typical filename could be like:
"%g_%b/%s_%g_%f_%i" --> apple2p_ap2000/20211223_0705_apple2p_sl1_uniprint_prn_ap2000_0000.png

or as simple as:
"%b/%i" -> ap2000/0000.png

@goldnchild
Copy link
Contributor Author

goldnchild commented Dec 25, 2021

One thing I just noticed:

copying the %t time format code from video_manager::open_next
it needs to be "char t_str[16];" to accommodate the format string since it's 4+2+2+1+2+2+2+1 (terminating zero) for 16 chars.

If it doesn't have enough room for the zero it will drop the last specifier:

char t_str[15]; and a -snapname "%g/%t" will give filenames like:
20211225_0406.png

char t_str[16]; and a -snapname "%g/%t" will give filenames like:
20211225_042939.png

      if (pos_time != -1)
        {
                char t_str[16];
                const std::time_t cur_time = std::time(nullptr);
                strftime(t_str, sizeof(t_str), "%Y%m%d_%H%M%S", std::localtime(&cur_time));
                strreplace(snapstr, "%t", t_str);
        }

@rb6502 rb6502 merged commit c21bea8 into mamedev:master Jan 7, 2022
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.

2 participants