Implement MQTT Extended Status Phase 1(a and b) #2007
Conversation
Implements the core infrastructure for MQTT extended status publishing as defined in issue #1938 Phase 1a. Changes: - Add generic publish() method to MQTTClient for flexible MQTT publishing - Create MQTTMediaStatusService following established service pattern - Add camera-state-changed and microphone-state-changed to IPC allowlist - Initialize mediaStatusService in main app when MQTT is enabled - Update IPC documentation with new channels (now 42 total) - Update research document with Phase 1a completion status What's working: - Call state publishing via existing call-connected/call-disconnected events - Infrastructure ready for camera/microphone monitoring (Phase 2) - Generic publish() method supports all future MQTT use cases Next steps: - Phase 1b: Document configuration schema for semantic categories - Phase 2: Implement WebRTC monitoring for camera/mic state detection Related: #1938
| ✅ Changelog entry generated and committed to this PR: The file You can edit it directly in this PR if needed. |
Summary of ChangesHello @IsmaelMartinez, 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! This pull request implements the foundational infrastructure for publishing extended MQTT status information, specifically focusing on media and call states. It introduces a new service to handle these events and a generic MQTT publishing mechanism, laying the groundwork for future enhancements like real-time camera and microphone status monitoring. Highlights
Using Gemini Code AssistThe 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 by creating a comment using either
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 Limitations & Feedback Gemini Code Assist 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. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. 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. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request lays a solid foundation for extended MQTT status publishing by introducing a new MQTTMediaStatusService and a flexible, reusable publish method in the MQTTClient. The code is well-structured and follows existing patterns in the codebase. However, there's a critical issue with the IPC handling for call states that will prevent that part of the feature from working. Additionally, there are some minor inconsistencies in the auto-generated documentation regarding IPC channel counts that should be corrected to prevent confusion.
docs-site/docs/development/research/mqtt-extended-status-investigation.md Show resolved Hide resolved
📦 PR Build Artifacts✅ Build successful! Download artifacts:
|
Fix conflict where call-connected and call-disconnected IPC channels were registered with both ipcMain.handle (browserWindowManager) and ipcMain.on (mediaStatusService), which is not allowed in Electron. Solution: - browserWindowManager emits internal events after handling IPC - MQTT service listens to internal events instead of IPC channels directly Changes: - browserWindowManager now emits 'teams-call-connected' and 'teams-call-disconnected' internal events after handling IPC - mediaStatusService listens to internal events, not IPC channels - Maintains existing IPC contract (still returns boolean) - No breaking changes to renderer-side IPC callers This decouples the event handling and allows multiple listeners for the same logical event.
Move internal call state events from ipcMain to app EventEmitter to properly separate IPC channels from internal process events. Changes: - Use app.emit() instead of ipcMain.emit() for internal events - MQTT service uses app.on() instead of ipcMain.on() for internal events - Add descriptive comments above all IPC channel registrations - IPC documentation now correctly shows 40 channels (was 42) - Remove duplicate call-connected/call-disconnected entries - All channel descriptions now present This fixes the IPC documentation issues identified by review: - Eliminates duplicate channel entries - Correct channel count (38 + 2 new = 40) - Complete descriptions for all channels
IsmaelMartinez force-pushed the claude/mqtt-extended-status-phase-1a-01NwR55q278TNrwhymeNFKUq branch from fcee25b to 5203977 Compare | ✅ Changelog entry generated and committed to this PR: The file You can edit it directly in this PR if needed. |
| Hello @vbartik, The changes in this branch should add the
Can you test it? Seems to work for me. Once this is out we can look at phase 2 that is adding the mic and camera status, that might be a bit more difficult. I still need to update the docs etc, but appreciate feedback to see if this is going to work. Thanks! |
IsmaelMartinez changed the title Document new MQTT topics in configuration guide. Changes: - Add "Published Topics" table to MQTT Integration section - Document {topicPrefix}/in-call, camera, and microphone topics - Explain payload format and retained message behavior - Mark Phase 1b as complete in research document - Update overall status to "Phase 1 Complete" All MQTT topics are now documented for users setting up home automation integrations. MDX interprets {topicPrefix} as JavaScript expressions. Escape them with backslashes to render as literal text. Fixes documentation build errors in configuration.md and mqtt-extended-status-investigation.md 
| Hi @IsmaelMartinez , I did some preliminary tests. The feature, in simple scenario - "clicked" join + "clicked" leave - works.
|
| Hi @vbartik thanks for the testing and review. A few comments back.
For 2 and 3 we could add an event that triggers indicating the app has loaded (and/or crashed), and you couldn't reset the status on your side. We could add extra events but I tend to prefer to keep them separated and to not emit events without a cause-effect. Thanks again for the test. I will think about it a bit more. Appreciate the feedback. |
| Hi @IsmaelMartinez , regarding 1+2, understood.
Maybe a MQTT LWT message could work here well?
You're right about the granularity. It allows granular event driven flows through targeted mqtt topic subscriptions, and you then don't really have to determine what changed within one big json. 👍 Aggregating into a single topic of multiple metrics would make sense when you're for example reading multiple values from a physical sensor at once, or similar. These are decoupled. |
| sorry, what do you mean with LWT ? (and apologies as I am extremely busy so playing catchup) |
| It is MQTT Will Message (Last Will & Testament = LWT). Basically, if a clients provides MQTT lastWill to the broker, upon the client's unexpected disconnection, the broker acts upon it by posting the lastWillMessage to the lastWillTopic. That way, you can update the status to 'disconnected' for example, even if you crash/lose connectivity/whatever unexpected happens. |
Implement MQTT LWT to handle app crashes and network failures gracefully. Changes: - Configure LWT on connection: publishes connected=false if app disconnects unexpectedly - Publish connected=true when successfully connected to broker - Publish connected=false on graceful disconnect - Add {topicPrefix}/connected topic to documentation How it works: - On connect: broker stores LWT message (connected=false) - App publishes: connected=true (while running) - App crashes/network fails: broker auto-publishes LWT (connected=false) - Graceful exit: app publishes connected=false before disconnecting This allows home automation to detect stale state and invalidate in-call/camera/mic status when the app is offline. Related: #1938 Document the implementation of MQTT Last Will and Testament. Changes: - Mark Phase 1c as completed in checklist - Add implementation notes explaining LWT behavior - Include Home Assistant example for handling disconnects - Update MQTT topics list to include connected topic - Update status header to reflect complete Phase 1 Phase 1 is now complete with: - 1a: Core infrastructure - 1b: Documentation - 1c: Connection state & LWT
|
| That LWT is now associated to a new topic "{topicPrefix}/connected" (teams/connected by default). Apologies but I got little time lately. Let me know if you are happy with the it and I can see if we can release this. |
Implements the core infrastructure for MQTT extended status publishing as defined in issue #1938 Phase 1a and 1b.
Changes:
What's working:
Next steps:
Related: #1938