Skip to content

D88エミュレーション実装 #12

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

Open
wants to merge 3 commits into
base: develop
Choose a base branch
from

Conversation

ponzu0147
Copy link
Owner

D88エミュレーション実装

実装内容

  • DriveEmulatorクラスの実装
    • ドライブの物理的な動作(ヘッド移動、回転待ち)のエミュレーション
    • タイミング制御の実装
    • エラー処理の追加
  • D88DiskExtensionの追加
    • セクタ読み書き機能の拡張
    • エラーハンドリングの改善

技術的な詳細

  • ヘッド移動時間: 3ms/track
  • ヘッドロード時間: 35ms
  • セトリング時間: 15ms
  • 回転速度: 300rpm
  • インターリーブ対応

Copy link

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Hello @ponzu0147, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

Summary of Changes

This pull request implements D88 disk emulation for the PMD88 iOS application. It includes the implementation of the DriveEmulator class, which emulates the physical actions of a disk drive (head movement, rotation waiting), timing control, and error handling. It also adds a D88DiskExtension to extend sector read/write functionality and improve error handling. The implementation details include head movement time, head load time, settling time, rotation speed, and interleave support.

Highlights

  • DriveEmulator Class: Implements the DriveEmulator class to simulate physical disk drive operations, including head movement and rotation waiting, with timing control and error handling.
  • D88DiskExtension: Adds D88DiskExtension to extend sector read/write functionality and improve error handling within D88 disk images.
  • FM Engine Enhancements: The FM engine is enhanced with key on state management, frequency calculation, waveform generation, and envelope calculation, improving the accuracy and richness of FM sound synthesis.

Changelog

Click here to see the changelog
  • AudioEngine.swift
    • No functional changes; the file was modified but the code content remains identical.
  • FMEngine.swift
    • Added properties for phases, envelopes, and channel key on state to the FMEngine class.
    • Implemented updateRegisters to detect key on state changes and update channel states.
    • Improved generateSample method to synthesize channel outputs based on active channels, phase, envelope, and volume.
    • Added methods to determine channel activity (isChannelActive), get channel FNUM and BLOCK values, calculate frequency, generate waveforms, calculate envelopes, and get channel volume.
  • PMD88iOS.xcodeproj/project.pbxproj
    • No functional changes; the file was modified but the code content remains identical.
  • PMD88iOS.xcodeproj/project.xcworkspace/contents.xcworkspacedata
    • No functional changes; the file was modified but the code content remains identical.
  • PMD88iOS.xcodeproj/xcuserdata/koshikawamasato.xcuserdatad/xcschemes/xcschememanagement.plist
    • No functional changes; the file was modified but the code content remains identical.
  • PMD88iOS/Assets.xcassets/AccentColor.colorset/Contents.json
    • No functional changes; the file was modified but the code content remains identical.
  • PMD88iOS/Assets.xcassets/AppIcon.appiconset/Contents.json
    • No functional changes; the file was modified but the code content remains identical.
  • PMD88iOS/Assets.xcassets/Contents.json
    • No functional changes; the file was modified but the code content remains identical.
  • PMD88iOS/BoardType.swift
    • No functional changes; the file was modified but the code content remains identical.
  • PMD88iOS/Constants.swift
    • No functional changes; the file was modified but the code content remains identical.
  • PMD88iOS/ContentView.swift
    • No functional changes; the file was modified but the code content remains identical.
  • PMD88iOS/D88Disk.swift
    • No functional changes; the file was modified but the code content remains identical.
  • PMD88iOS/D88DiskExtension.swift
    • Added readSectorData function to read sector data from a D88 disk image.
    • Added writeSectorData function to write sector data to a D88 disk image.
    • Added isWriteProtected function to check if the D88 disk is write protected.
    • Added getTrackInfo function to retrieve track information from a D88 disk image.
    • Added getSectorInfo function to retrieve sector information from a D88 disk image.
  • PMD88iOS/DriveEmulation.swift
    • Introduced DriveTimings struct to define timing constants for disk drive operations.
    • Defined DriveState struct to manage the state of the emulated drive.
    • Created SectorAccess struct to handle sector access control and interleave calculations.
    • Defined DriveError enum to represent various drive-related errors.
    • Implemented D88DriveEmulator class to emulate disk drive behavior, including motor control, head movement, and sector read/write operations.
  • PMD88iOS/Engine/ADPCMEngine.swift
    • No functional changes; the file was modified but the code content remains identical.
  • PMD88iOS/Engine/AudioEngine.swift
    • No functional changes; the file was modified but the code content remains identical.
  • PMD88iOS/FMAlgorithm.swift
    • No functional changes; the file was modified but the code content remains identical.
  • PMD88iOS/FMEnvelope.swift
    • No functional changes; the file was modified but the code content remains identical.
  • PMD88iOS/FMTypes.swift
    • No functional changes; the file was modified but the code content remains identical.
  • PMD88iOS/PC88/PC88Audio.swift
    • No functional changes; the file was modified but the code content remains identical.
  • PMD88iOS/PC88/PC88Core.swift
    • No functional changes; the file was modified but the code content remains identical.
  • PMD88iOS/PC88/PC88Debug.swift
    • No functional changes; the file was modified but the code content remains identical.
  • PMD88iOS/PC88/PC88PMD.swift
    • No functional changes; the file was modified but the code content remains identical.
  • PMD88iOS/PC88/PC88Types.swift
    • No functional changes; the file was modified but the code content remains identical.
  • PMD88iOS/RhythmEngine.swift
    • No functional changes; the file was modified but the code content remains identical.
  • PMD88iOS/SSGEngine.swift
    • No functional changes; the file was modified but the code content remains identical.
Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.

Feature Command Description
Code Review /gemini review Performs a code review for the current pull request in its current state.
Pull Request Summary /gemini summary Provides a summary of the current pull request in its current state.
Comment @gemini-code-assist Responds in comments when explicitly tagged, both in issue comments and review comments.
Help /gemini help Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist is currently in preview and may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments to provide feedback.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.


In realms of code, where bits align,
A D88 disk, a treasure to find.
Emulation's art, a digital quest,
To bring old sounds to modern test.

Footnotes

  1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution.

Copy link

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request implements D88 disk emulation, including drive physics, timing control, and sector read/write functionality. The code appears well-structured and addresses the requirements outlined in the pull request description. However, there are some areas that could benefit from further review and potential improvement, particularly in error handling and code clarity.

Summary of Findings

  • Incomplete Error Handling in D88DiskExtension.swift: The D88DiskExtension.swift file lacks proper error handling for dataOverrun errors during sector data writing. This could lead to unexpected behavior or data corruption if the provided data exceeds the sector's capacity. A more robust error handling mechanism should be implemented to prevent this.
  • Missing Error Handling in D88DriveEmulator.swift: The D88DriveEmulator.swift file lacks comprehensive error handling in the readSector and writeSector functions. While some errors are thrown, there's no mechanism to catch and handle them gracefully, potentially leading to unhandled exceptions and program crashes. Consider adding do-catch blocks to manage these errors.
  • Potential for Deadlock in Audio Engine: The AudioEngine.swift file uses completeStop which calls resetAllEngines which contains a call to updateState on the audio engines. This could lead to a deadlock if the audio engine is already in the process of updating its state on another thread. Consider refactoring to avoid this potential deadlock.

Merge Readiness

The pull request introduces significant functionality for D88 disk emulation. However, due to the presence of high and medium severity issues, it is not recommended to merge this pull request until these issues are addressed. Ensure that error handling is robust and potential deadlocks are resolved before merging. I am unable to directly approve this pull request, and users should have others review and approve this code before merging.

Comment on lines +149 to +150
// 音源エンジンのリセット
resetAllEngines()

Choose a reason for hiding this comment

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

medium

The completeStop function is called within stop and start which then calls resetAllEngines which then calls updateState on the audio engines. This could lead to a deadlock if the audio engine is already in the process of updating its state on another thread. Consider refactoring to avoid this potential deadlock. This is a medium severity issue because it could lead to the application hanging.

    // 完全停止処理
    private func completeStop() {
        // 現在のプレイヤーノードを停止
        if let player = playerNode {
            // 再生中かどうかに関わらず強制的に停止
            player.stop()
            
            // バッファをリセット
            player.reset()
            
            // エンジンから切り離す
            engine.detach(player)
            playerNode = nil
            print("🔊 プレイヤーノード解放")
        }
        
        // エンジンを停止
        do {
            // エンジンの実行状態に関わらず強制的に停止
            engine.stop()
            print("🔊 AVAudioEngine停止")
            
            // エンジンを完全にリセット
            engine.reset()
            print("🔊 AVAudioEngineリセット完了")
            
            // オーディオセッションを非アクティブにする
            try AVAudioSession.sharedInstance().setActive(false)
            print("🔊 オーディオセッション非アクティブ化")
        } catch {
            print("⚠️ オーディオエンジン停止エラー: \(error)")
        }
    }

@ponzu0147
Copy link
Owner Author

やりましょう

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.

1 participant