[MichaIng]

• The record start/stop action was never implemented, just stray dead code. There is no easy way to implement it, since motion has no recording-only action/API, but still locks the camera devices, so that a manual FFmpeg invocation runs into a "Device or resource busy" error.
• Instead, a default-off switch for a new action button to start/stop a motion event manually has been added. It makes use of the previously implemented new eventstart/eventend actions. This comes as close to a recording button as it gets, triggers the whole motion event notification pipeline, but at least stops recordings immediately, without post_capture.
• The snapshot action path and function has been merged with the other two, since they do identical API calls. The API also returns a 400 now, if snapshot_filename is not set, since otherwise motion crashes.
• While on it, a default-off switch for a new action button to quickly toggle motion detection, without a full settings change, has been added, too, along with the needed action API extension. The motionctl function existed already, as the same API was used to disable motion detection prior to motion v4.4, and by the working schedule feature. Its argument has been adjusted to reflect, that it is makes use of motion's detection/pause and detection/start API, for temporarily switching the state, instead of being a persistent setting, like the motion >=4.4 pause config key, and our related @motion_detection. The state is cached, since it is obtained by every picture/current call of the frontend to refresh the camera frames, like the motion event state. This means, that manually calling the motion API to pause/start motion detection, can cause a wrong state across motionEye. But I guess this is an acceptable regression.
• The other motionctl functions have been aligned to handle AsyncHTTPClient().fetch() errors correctly. They all threw an exception in any case of error. raise_error=False is used to set resp.error instead for actual HTTP response errors, a body check for motion "Bad Request" (HTTP 200) responses, and a try-except for other kind of connection errors, to properly log and return them to the client, instead of throwing exceptions.
• Tests have been added to cover the 5 action API points, and 2 new settings.

[MichaIng]

I played a bit around:

• With emulate_motion, pre/post capture is appended to the movie. event_gap however is not.
• With eventstart, neither pre/post capture, not event_gap are added to the movie. So it is closer to a start/stop recording.
• Both trigger the full motion event notification pipeline.
• emulate_motion requires webcontrol_parms = 1, eventstart works with webcontrol_parms = 0.
• Neither can be used for a manual-only recording mode, since there is no equivalent to the snapshot settings and API to manually take pictures, without having them taken on any motion event.
• Of course it is possible to pause motion detection, but tying manual recordings with general motion detection isn't great.
• A true manual-only start/stop recording would require invoking FFmpeg on the video device/network stream manually. But motion occupies camera devices, even if motion detection is paused. FFmpeg fails with "Device or resource busy".

So overall, I see no simple way to implement a true manual recording API + buttons. But instead, we can implement a manual motion event toggle API and button. And while on it, the same for pausing/resuming motion detection. Both as new default-off toggles in the motion detection section.

[JamBalaya56562]

snapshot goes to motion without checking get_action_commands(), so it is sent even when the camera has no snapshot_filename and the button is hidden. motion 4.7.1 then crashes: /1/action/snapshot answers "Done", then the process is gone and motionEye restarts it, so all cameras drop. Any logged-in user can post actions. This is not new, but as this PR merges the snapshot path: maybe answer 400 unless get_action_commands() has snapshot?

[MichaIng]

Good point. That was the same before, the action API never checked that, but makes sense to do so. I'm adding this to the tests as well. get_action_commands() checks too much, as the button does not need to be enabled in the frontend for the action API to work, only still images need to be enabled, i.e. snapshot_filename set in the config.

[MichaIng]

Ah, I btw want to change the SVGs as well, once everything else is sane. Need to think about a good simple symbol to represent a motion event, and motion detection respectively. On/off could be visualized with inverted colors, like some other buttons do, and/or a cross to be unambiguous.

[Marijn0]

Looks good to me, quite nice additions, I like them a lot!

[JamBalaya56562]

I tried 4b505e5 in Docker (motion 4.5.1). Tests pass, and the buttons work as described. Two questions and two small things:

• With a working schedule, the pause doesn't last: with "during" set to all day, detectionpause gave PAUSE, and 25 s later it was ACTIVE again. wsswitch sends start every 10 s while @motion_detection is on. Should wsswitch leave a paused camera alone, or should the tooltip mention it?
• A normal user gets 403 on /config/1/set, but detectionpause/detectionstart return 200 and change the motion state. Is that intended for a view-only user, or should these be admin only?
• Connection errors aren't caught: with nothing on the control port, get_motion_detection, set_motion_detection and trigger_action raise ConnectionRefusedError (tornado 6.5.10). Since tornado 6, raise_error=False only covers HTTP errors, and this isn't an HTTPClientError. Catching OSError too would do.
• The cache outlives a motion crash: after a pause, I killed motion and the checker started it again (ACTIVE), but motion_detection_1 was still false. The checker calls start() without stop(), so clearing _motion_detection in start() would fix it.

[MichaIng]

Regarding wsswitch: Hmm, it would need to differentiate between a manual state change done via button/API, and the intended state change based on motion detection setting and working schedule times, somehow. Not sure how to implement this, since wsswitch uses the same motionctl function. What would be probably optimal:

• If motion detection is enabled due to wsswitch times, and gets disabled by something else, wsswitch should not automatically re-enable it.
• Similarly, if motion detection is disabled due to wsswitch times, and gets enabled by something else, wsswitch must not disable it.
• A change from wsswitch active/inactive times should reset the "manual change" flag.

The best I can think of is a wsswitch cache:

• It stores the state it actively set.
• As long as the intended state does not change, due to transition between active and inactive times, it does not check or touch motion detection, but just assumes it remains as intended, or otherwise was intentionally changed and should remain like that.
• When there is a transition between active and inactive times, i.e. its cached state does not match the intended state, it enforces the intended state, and caches that again.
• On motionEye startup, either the camera config (motion >=4.3) or motionctl sets the initial motion detection state, based on whether it is enabled via motionEye config or not. The wsswitch loop starts afterwards. As its cache is empty at that time, it will once enforce and cache the intended state.
• One issue is motion crashes or stop/start cycles. wsswitch needs to re-apply the intended state in this case, hence motionctl.start() would at best need to reset the wsswitch cache. It would however need to avoid a circular import, i.e. that would need to be done in the motionctl.start() function itself, e.g. from wsswitch import clear_cache.

That would also significantly reduce processing of the loop. We could further start the loop only if working schedule is actually enabled in any active camera config, turn it into a cached PeriodicCallback, and start/stop that based on settings on every config save.

But I'd do all that in 1-2 separate PR. For now, I am fine with letting wsswitch override the state every 10 seconds, the way it always did.

---

Regarding normal user permissions: The action API itself does and did not restrict it, e.g. for snapshots neither. It is and was the admin which decides which buttons to enable, and which action scripts to create, and the normal user can and could use them. I wouldn't change that for now, as it would introduce some inconsistency. But we can re-evaluate in a dedicated issue/PR whether all or some action buttons require stricter permissions, or maybe an admin-only flag per action button 🙈.

---

Regarding the ConnectionRefusedError: Then we should maybe just catch that particular one, instead of OSError. I wouldn't want to gracefully handle a too wide range of exceptions, but only those caused by an actual server or network error. E.g. if there is some serious client-side bug, broken system state, or any such, I'd prefer an unhandled exception.

---

Good catch about the issue not clearing the cache on motion start. Makes sense, makes it more robust, and costs nothing. Done in 8d6ad46.

[JamBalaya56562]

Re-tested 8d6ad46: after a motion crash and restart by the checker, the detection button now follows the real state, and the tests pass. Fine with the working schedule and permissions in separate PRs; the ConnectionRefusedError catch can go here or in a follow-up, as you prefer.

[MichaIng]

Did let Copilot create some example icons for the two buttons and each state, following the simple SVG design of the other buttons. Which ones do you prefer? #3498

I'd probably go with:

• Start motion detection:
• Pause motion detection, adding some more eyelashes and flatter curve:
  EDIT: That is better:
• Start motion event:
• Stop motion event, making the horizontal lines slightly thicker (it is actually fine):

But I would be happier with more specific motion event start/stop icons, instead of these generic start/stop anything. Maybe the pedestrian traffic light walking/standing human icons, used in some countries.
EDIT: First attempt not satisfying for me 😄:

EDIT2: Better, but I'm letting Claude try its best for another refined version:

One side note: The action button icons represent what pressing the button does, not what the current state is. That can be actually confusing, but it is how the other buttons are designed, and their classes named, and how the recording button was intended to work. I am thinking now that the title text (hover text) needs to be adjusted, to say what pressing the button does, instead of "toggle ...". I'll change that later.

Not bad:

Top is rendered at 40x40 pixels, i.e. their size in the UI. The 2nd column interestingly matches the Copilot result exactly, when I gave both the same sample to compare against. The 3rd column is what I'd take, since the red/green color could cause some unintended implicit associations: We do not want to give anyone an alarm signal with a red colored icon. EDIT: Ah, the 4th column is just a blend of sample and SVG, to see how well it matches.

[Marijn0]

Which ones do you prefer?
The 3rd column is what I'd take

Yes, I agree. 👍

These also look good!

[MichaIng]

I added them here, i.e. open/closed eye for motion detection button, walking/standing figure for starting/ending motion event. But still open for suggestions, if you prefer any other variant from #3498, or another idea, or some refinement is needed. Whether the the SVGs are a KiB larger or smaller really does not matter either.

EDIT: Ah I see you agreed already. Great, also the changing hover text is in now. Doing a last test of that before merge.

[Marijn0]

Tested again and everything works great!

One thing I noticed: I think we can improve how these buttons are arranged, but that's something for a separate PR. With 2 or more action buttons they only line up correctly after I open the settings menu, which seems to trigger the recalculation of how many rows are needed to display them. I think you're seeing the same thing, right? 😅

[MichaIng]

What I recognized earlier, unrelated to this or any recent commit, is that the action buttons are sometimes ordered horizontally until the layout is updated by e.g. switching back and forth between single cam view, toggling the config panel, or similar. Buttons can then be outside of the frame, hence hidden. E.g. here the motion detection button is below the motion event button, outside the frame:

But can be fixed in a dedicated PR.